Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spec for Triggers and Custom Clickable Links #15700

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

zadjii-msft
Copy link
Member

@zadjii-msft zadjii-msft commented Jul 12, 2023

Abstract

This spec outlines a mechanism by which users can define custom actions to run
when a string of text is written to the Terminal. This lets users create
powerful ways of automating their Terminal to match their own workflows. This
same mechanism can be used by third-party applications to customize the way the
terminal control automatically identifies links or other clickable regions of
the buffer, and handle it in their own way.

TODO

@zadjii-msft zadjii-msft added the Issue-Docs It's a documentation issue that really should be on MicrosoftDocs/Console-Docs label Jul 12, 2023
@github-actions

This comment has been minimized.

@github-actions
Copy link

@check-spelling-bot Report

🔴 Please review

See the 📂 files view or the 📜action log for details.

Unrecognized words (1)

sustainability

Previously acknowledged words that are now absent homeglyphs 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 git@github.com:microsoft/terminal.git repository
on the dev/migrie/s/5916-draft branch (ℹ️ how do I use this?):

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 this

By 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.

⚠️ The command is written for posix shells. If it doesn't work for you, you can manually add (one word per line) / remove items to expect.txt and the excludes.txt files.

If the listed items are:

  • ... misspelled, then please correct them instead of using the command.
  • ... names, please add them to .github/actions/spelling/allow/names.txt.
  • ... APIs, you can add them to a file in .github/actions/spelling/allow/.
  • ... just things you're using, please add them to an appropriate file in .github/actions/spelling/expect/.
  • ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in .github/actions/spelling/patterns/.

See the README.md in each directory for more information.

🔬 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 positives

If items relate to a ...

  • binary file (or some other file you wouldn't want to check at all).

    Please add a file path to the excludes.txt file matching the containing file.

    File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

    ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

  • well-formed pattern.

    If you can write a pattern that would match it,
    try adding it to the patterns.txt file.

    Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

    Note that patterns can't match multiline strings.

Comment on lines +172 to +174
`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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 😝

Comment on lines +153 to +156
* `"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
Copy link
Member

@lhecker lhecker Jul 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean it would match 1 line below the viewport? What does an anchor of "bottom" mean and what are the others?

@lhecker
Copy link
Member

lhecker commented Jul 13, 2023

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

VsCode only runs matchers on prompt sequences, for performance reasons.

and we could do that as well initially and forego parts of the settings object and logic needed.

@zadjii-msft
Copy link
Member Author

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

Oh in my prototype I did some real dumb stuff:
https://github.com/microsoft/terminal/compare/f99757176215e46b76b82d0bd4baa5a95f76a5e6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Docs It's a documentation issue that really should be on MicrosoftDocs/Console-Docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants