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

Triggers(Including Text Strings) and Actions (internal or external calls) #5916

Open
fischerdouglas opened this issue May 14, 2020 · 8 comments
Labels
Area-Extensibility A feature that would ideally be fulfilled by us having an extension model. Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Terminal The new Windows Terminal.
Milestone

Comments

@fischerdouglas
Copy link

fischerdouglas commented May 14, 2020

Description of the new feature/enhancement

A Trigger system that will monitor several types o events, including text strings on the screen, that will be able to call several types of actions, like scripts, commands, or any resources possible of Windows Terminal.

Proposed technical implementation details (optional)

"Nothing is created, everything is copied."
As you you may see, I'm a fan of iTerm2, my buddy for several years.
But I like this new Microsoft Style, so I'm trying to help.

The Suggestion her is a bit bold.
Provide an complementar Framework that will allow user execute almost any possible predefined action based on almost any event on the Windows Terminal.

A sub-system the will be monitoring events, and if it matches with some pre-activated event, it will trigger the action.

The triggers could be a simple match with a string on the text, or even something like an server-ended-session if it was an SSH/Telnet connection.

The actions could be the automatic highlight of the text, a long beep, open a new tab, or call an external app passing some parameters and receiving back some answer.


As I'm not a developer, and also not good with descriptions like these, I will bring the description of this Feature in iTerm2:
https://www.iterm2.com/documentation-one-page.html#documentation-triggers.html

@fischerdouglas fischerdouglas added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label May 14, 2020
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels May 14, 2020
@WSLUser
Copy link
Contributor

WSLUser commented May 15, 2020

This sounds like a dupe of some other issue dealing with clickable links and scripts...a great extension idea though if it's done like it is in Iterm2.

@zadjii-msft zadjii-msft added Area-Extensibility A feature that would ideally be fulfilled by us having an extension model. Product-Terminal The new Windows Terminal. labels Jun 9, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jun 9, 2020
@zadjii-msft zadjii-msft added this to the Terminal Backlog milestone Jun 9, 2020
@zadjii-msft zadjii-msft changed the title Triggers(Including Text Strings) and Actions (internal ou external calls) Triggers(Including Text Strings) and Actions (internal or external calls) Jun 9, 2020
@zadjii-msft zadjii-msft removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jun 9, 2020
@zadjii-msft
Copy link
Member

x-linking with #12366.

Maybe it needs to be closer to

"triggers": [
    { 
        "type": "matchRegex",
        "match": "    git push --set-upstream origin (.*)",
        "command": { "action": "sendInput", "input": "git push --set-upstream origin ${match[0]}" }
    }
]

From iterm2 docs

Triggers have a regular expression, an action, an optional parameter, and may be marked as Instant

so that would cover

  • a regular expression
  • an action
  • I believe the ${match[0]} thing I have there is vaguely the "optional parameter" thing
  • "Instant" seems easy enough as a bool, though I'd probably want to play with it in iterm2 to get a more precise feel

<hackathon thoughts>
These actions would need to be stored as raw json and then evaluated when we want to swap out the ${match[0]} bits. That's probably annoyingly painful perf wise. We'd also need to like, pass the regexes as indexes to the Control, who then raises an event saying "regex 1 happened". Awkward. Plus there's the whole JSON escaping problem then, don't want the match exiting the json string.

But, there's also the concept of the right-click context menu that likely needs to evaluate the actions json when the right click happens, to populate entries accordingly. I suppose these would come with caveats that "using many triggers may substantially impact performance"

@zadjii-msft
Copy link
Member

But maybe it shouldn't just be all actions.

Cause, the control and the actual terminal core are[1] separated across a process boundry. So, anything that processes raw output from the connection should be handled within the core itself, or be pretty substantially throttled.

So for something like:

"triggers": [
    { 
        "type": "matchRegex",
        "match": "[Ee][Rr][Rr][Oo][Rr]",
        "command": { "action": "addMark", "color": "#ff0000" }
    }
]

the action would either need:

  • to be handled in the ControlCore itself, implying there are control actions, that don't rely on TerminalSettingsModel
  • to be passed position information in some way, so it could refer to the position the match occurred at even after potentially more text has been output.
    • So maybe ${match} has the sections of text that were matched, and ${matchStart}, ${matchEnd} get evaluated as the positions of the match in the buffer. But even that is problematic. The Core would need to track where the match was in the buffer until the match scrolls out. And the ControlCore would need to be the one that actually evaluates that variable, because there could be more scrolling that occurs between the action being executed in the UI and the content process actually handling it.

After typing that up, the first seems easier, but comes with some limitations. Like, you wouldn't be able to request a new tab on a trigger. Probably fine, but meh. Cause globalSummon as the action might be fun

[1]: will be* technically, but whatever I'm almost done with that work so let's work on that assumtion.

@zadjii-msft
Copy link
Member

<showerthought>

#8849, #6969

A trigger action could be "turn the matched text into a clickable link". The tricky bit is that those issues have lots of variations:

  • Open a webpage to the marked area. So literally just insert a hyperlink into the matched area.
  • open up an editor to the file (presumably ShellExecute('code.exe ${match}') or similar)
  • search web for highlighted text feature? #10175 mentions matching errors to automatically generate links to "Search StackOverflow" or similar. That one's maybe a little more tricky to actually generate a button, but turning them into links isn't a terrible idea.
  • Maybe you just want to be wild and match a path to a sendInput('cd "${match}") action! That would be wild and awesome
    • obviously insane, cause something like d:\dev\public\terminal would need to have like, 4 overlapping links and I'm not sure that the hyperlinks can overlap.

#9583 could help power this though

@zadjii-msft
Copy link
Member

dev/migrie/fhl/5916-triggers (specifically e1bd3e9) is where I realized that this needs a slightly different impl. Something like, control actions, for sure. Here's the skivy.

Even if we bubble up "we hit trigger 5" to the pane, and the pane generate the ActionAndArgs, and then tosses that to ShortcutActionDispatch, ShortcutActionDispatch is gonna toss away the context! Like, if we do the example:

            "experimental.triggers": [
                {
                    "type": "matchRegex",
                    "match": "Terminate batch job \\(Y/N\\)",
                    "command": { "action": "sendInput", "input": "y\n" }
                }
            ]

then we've got a background pane that triggers it, and oh no! the active pane gets some y\n keypresses. sad.

I wonder if there's a non-dumb way to refactor actions s.t. we could have certain actions that operate on a pane, and call a dispatch with a pane context. Hmm.

@zadjii-msft
Copy link
Member

Oh, that's annoying though, cause this doesn't work nicely with Terminate batch job (Y/N)? actually - cmd doesn't print that with a trailing newline. It just prints that and waits there. Maybe it's not the worst impact to add this both

  • on a newline
  • at the end of a WriteBuffer (e.g. a frame from conpty)

@zadjii-msft
Copy link
Member

zadjii-msft commented Aug 17, 2022

gh-5916-prototype-000

This would surely cause a definite perf hit, but hey maybe you're the kind of person who doesn't care about that too much?

This is at the very least, a pretty compelling PoC

image
gh-5916-prototype-001

            "experimental.triggers": [
                {
                    "type": "matchRegex",
                    "match": "Terminate batch job \\(Y/N\\)",
                    "command": { "action": "sendInput", "input": "y\r\n" }
                },
                {
                    "type": "matchRegex",
                    "match": "[Ee][Rr][Rr][Oo][Rr]",
                    "command": { "action": "addMark", "color": "#ff0000" }
                },
                {
                    "type": "matchRegex",
                    "match": "'(.*)' is not recognized as an internal or external command,",
                    "command": { "action": "sendInput", "input": "winget-install ${match[1]}" }
                },
            ]

@zadjii-msft zadjii-msft modified the milestones: Icebox ❄, Backlog Aug 17, 2022
@zadjii-msft
Copy link
Member

zadjii-msft commented Aug 26, 2022

Note from trying to fall asleep last night:

  • It doesn't need to be every action that bubbles up and down. NewTab doesn't require context of where the match was. But marks, selection, input(maybe) might.
  • type is redundant above. Instead, lets shim some "actions" in there
    • {
          "match": "[Ee][Rr][Rr][Oo][Rr]",
          "type": "addMark",
          "color": "#ff0000"
      },
    • {
          "match": "Terminate batch job \\(Y/N\\)",
          "type": "sendInput",
          "input": "y\r\n"
      },
    • {
          "match": "'(.*)' is not recognized as an internal or external command,",
          "type": "raiseAction",
          "command": { "action": "checkWinGet", "commandline": "${match[1]}" }
      },
    • {
          "match": "This will open a new Terminal tab",
          "type": "raiseAction",
          "command": { "action": "newTab" }
      },

addMark, sendInput, etc, those can all be handled w/in the control, on the out thread. They'll just do internally, with the necessary context.

A raiseAction comes with the implication that the immediate context might be lost. So, a rasieAction(addMark) is a bad idea.


Could I be even crazier:

{
    "match": "[Ee][Rr][Rr][Oo][Rr]",
    "action": "addMark",
    "color": "#ff0000"
},
{
    "match": "Terminate batch job \\(Y/N\\)",
    "action": "sendInput",
    "input": "y\r\n"
},
{
    "match": "git push --set-upstream origin ([^\\w]*)",
    "action": "sendInput",
    "input": "git push --set-upstream origin ${match[1]}\r"
},
{
    "match": "'(.*)' is not recognized as an internal or external command,",
    "action": "checkWinGet",
    "commandline": "${match[1]}"
},
{
    "match": "This will open a new Terminal tab",
    "action": "newTab"
},

triggers is an array of things. First we try to deserialize each thing as a Control.Trigger. There's only a few options, and fundamentally those are basically actions`.

  • addMark
  • sendInput
  • experimental.colorSelection

And we can probably reuse some of the macros in ActionArgs.h to make deserializing to a Control.Trigger easy.

Control.Trigger's, when they are hit by the control, will be able to string replace their args themselves. The git push trigger above - when matched, the Core will call Control.SendInputTrigger.Execute(ControlCore core, String[] matches) (or something like that). Execute will be responsible for string-replacing anything it needs to.

If We find that the action is NOT a Control.Trigger, then we'll make it into an Settings.Model.Trigger. We'll hang on to the JSON. When the trigger is hit by the control, it'll raise an event. The app will recieve the event, match it up to the trigger, and call Model.Trigger.Parse(String[] matches) -> ActionAndArgs which will do the string replace in the JSON itself. We'll then just take the whole json, and try to parse it literally as an ActionAndArgs.


I put the spec into dev/migrie/s/5916-draft

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Extensibility A feature that would ideally be fulfilled by us having an extension model. Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Terminal The new Windows Terminal.
Projects
Specification Tracker
  
Spec In Review ⏰
Development

No branches or pull requests

3 participants