Spec for Triggers and Custom Clickable Links#15700
Spec for Triggers and Custom Clickable Links#15700zadjii-msft wants to merge 18 commits intomainfrom
Conversation
This comment has been minimized.
This comment has been minimized.
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view or the 📜action log for details. Unrecognized words (1)sustainability Previously acknowledged words that are now absenthomeglyphs keyevent ONLCR OSCBG OSCCT OSCFG OSCRCC OSCSCB OSCSCC OSCWT testdlls Tlgdata xxyyzz :arrow_right:To accept ✔️ these unrecognized words as correct and remove the previously acknowledged and now absent words, run the following commands... in a clone of the [email protected]:microsoft/terminal.git repository curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/v0.0.21/apply.pl' |
perl - 'https://github.com/microsoft/terminal/actions/runs/5536057415/attempts/1'✏️ Contributor please read thisBy default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.
If the listed items are:
See the 🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉 If the flagged items are 🤯 false positivesIf items relate to a ...
|
| `match` also accepts just a single string, to automatically assume default | ||
| values for the other properties. For example, the JSON `"match": | ||
| "[Ee][Rr][Rr][Oo][Rr]"` is evaluated as |
There was a problem hiding this comment.
But why do that now? We don't even know yet how the feature will actually work in practice and thus don't really know if such shortcuts would be helpful either. Less ways to specify settings seems generally beneficial to me.
There was a problem hiding this comment.
My gut says that more often than not, I just want to match on a line of text as it is output, so I usually don't care about the rest of the match params. I wanted to make it easier for folks to write that into their settings, for folks who are editing by hand.
There was a problem hiding this comment.
I still think it'd be better to have just one way to specify settings, at least initially. But I didn't mean to imply that I'm against it either.
There was a problem hiding this comment.
ah see, I'd probably start with this string version first, then expand out to the object version as we add other properties in successive PRs 😝
| * `"anchor"` (_default: `"bottom"`_) : Which part of the viewport to run the match against. | ||
| - VsCode has this, but I don't think we really do. I think we can just always say bottom. | ||
| * `"offset"` (_default: `0`_) : Run the match starting how many rows from the anchor | ||
| * `"length"` (_default: `1`_) : How many rows to include in the match |
There was a problem hiding this comment.
Does this mean it would match 1 line below the viewport? What does an anchor of "bottom" mean and what are the others?
|
The spec in its entirety might be difficult / time-consuming to implement correctly. Our current URL regex code is still essentially a prototype and would need to be rewritten to work well. For instance, it still doesn't work well with text that is partially outside of the viewport. There's also the problem that we won't be able to correctly match newlines as written in the spec because we currently don't store any newlines. The same would be true for matching trailing whitespace. I feel like we should separate the parts of the spec that we want to implement and ship immediately from the parts that we'd do over time and put those into one or more "Future" section or similar. For instance, the spec says
and we could do that as well initially and forego parts of the settings object and logic needed. |
Oh in my prototype I did some real dumb stuff: |
|
Status? |
|
We can close this. I'm obviously preoccupied, and not coming back around to finish this any time soon. Any enterprising young contributor who wants to carry this forward is welcome to open their own version. |
TODO