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

Piping visible text to external programs #1615

Open
Deewiant opened this issue Oct 2, 2018 · 32 comments
Open

Piping visible text to external programs #1615

Deewiant opened this issue Oct 2, 2018 · 32 comments

Comments

@Deewiant
Copy link

Deewiant commented Oct 2, 2018

(This was previously mentioned in #113 (comment) by a now-deleted user but seems to have been lost in the noise, so I thought I'd write it up properly.)

There is a patch for st called externalpipe. It allows you to bind shortcut keys for piping the text from the terminal's screen to arbitrary external programs. I would very much like to have this functionality available in Alacritty.

The example there uses it as a workaround for URL recognition (#113) by piping to an external URL recognizer and calling dmenu to select among the options. I find this quite convenient, using the urifind tool from http://metacpan.org/pod/URI::Find. One could also use other tools to implement different kinds of pattern matching, thereby solving the use cases mentioned by @casey in #113 (comment) and #113 (comment). The main difference being that this would be a keyboard-driven instead of a mouse-driven interface.

In the end I would expect a configuration interface like the following in key_bindings (approximating what @casey's examples might look like using dmenu to pick among the matches visible on-screen):

key_bindings:
  - { key: F, mods: Control, externalpipe: { program: "sh", args: ["-c", "rg -o 'https://[^\\s]+' | dmenu | xargs firefox"] } }
  - { key: I, mods: Control, externalpipe: { program: "sh", args: ["-c", "rg -o '[-_0-9a-zA-Z/]+:[0-9]+' | dmenu | parallel -d: 'some-ide {1} --line {2}'"] } }
  - { key: G, mods: Control, externalpipe: { program: "sh", args: ["-c", "rg -o '#[0-9]+' | dmenu | xargs -i firefox https://github.com/jwilm/alacritty/issues/{}"] } }
  - { key: G, mods: Control|Shift, externalpipe: { program: "sh", args: ["-c", "rg -o '[a-f0-9]{40}' | dmenu | xargs -i firefox https://github.com/jwilm/alacritty/commit/{}"] } }

This seems like a simpler way of approaching URL recognition and similar issues, since with this approach Alacritty doesn't have to concern itself with performing the pattern matching, only providing a straightforward data dump. It may be a relatively quick way of having a solution that is good enough for a lot of people.

@chrisduerr
Copy link
Member

This feature has been requested multiple times, however in varying different contexts. I think for opening URLs a built-in way to resolve it is a better way to go, however I can still see some ways to use this for other purposes.

@Deewiant
Copy link
Author

Deewiant commented Oct 2, 2018

Yeah, I certainly didn't mean to pitch this as a full replacement for #113.

@chrisduerr
Copy link
Member

chrisduerr commented Oct 26, 2018

To implement this feature, it would first be required to add a function on the Grid to convert its content to a String. I'd propose just implementing the ToString trait for grid to achieve this.

Since the grid is basically a 2D array, the easiest way to iterate over it to collect it into a String, should be to have two nested loops to iterate over all cells, similar to what is done here:
https://github.com/jwilm/alacritty/blob/master/tests/ref.rs#L105-L113

Alternatively it would be possible to make use of the iterator which is implemented for Grid already, however I would expect that to produce less readable code since it would make checking for line endings a bit more difficult.

A nice detail would also be to check the last cell in a line for the WRAPLINE flag. If this is set, there shouldn't be a newline in between the two lines.

After the ToString trait has been implemented, the method needs to be exposed in the ActionContext trait in the input.rs file. After which it will be possible to bind it to a key event in the impl Action section in the input.rs file.

After the Grid can be converted to a String and the input.rs file has access to that method, it is necessary to find a way to bind this to a specific action in the configuration file. I see two ways to do this:

  • Extend the key_bindings section to allow for specifying an action plus the command to which the string should be sent
  • Create a new, separate option for piping the buffer and add a key and modifiers field to it

If there are any further questions, please let me know.

@expectocode
Copy link
Contributor

Another use case where this would be a nice feature is when you have an unexpectedly large output (eg a traceback) and you want to view it in a different program (eg vim).

For this kind of scenario, it would be more helpful to take the scrollback buffer, not just the visible portion.

@chrisduerr
Copy link
Member

For this kind of scenario, it would be more helpful to take the scrollback buffer, not just the visible portion.

I think it would be possible to offer both and allow the configuration to specify what exactly should be copied.

@nicklan
Copy link

nicklan commented Oct 1, 2019

Copying my comment over from #2568:

I find this is a generally useful function to have. It happens to address a couple of current pain points that currently exist, but I would use this even if alacritty supported searching.

Some examples of what I would use this for:

  • A quick way to get a log that was dumped to stdout that I want to paste into a ticket/comment/etc
  • When I need more rich searching (For complex search I imagine many users prefer their editor to whatever their terminal implements)
  • If I'm creating documentation it's nice to run a bunch of commands, then pop that into an editor so I can clean it up and save it

@chrisduerr
Copy link
Member

This might also help with #2792, depending on implementation.

@nicklan
Copy link

nicklan commented Oct 3, 2019

So this is basically already implemented at https://github.com/jynnantonix/alacritty/tree/v0.3.3-scrollback-search

I've been using it for a bit and it works fine. From looking at the above branch, it's really not a lot of extra code, doesn't seem like it will be a big maintenance burden, and is 100% opt-in.

So my question would be, what are the arguments against implementing this?

@nixpulvis
Copy link
Contributor

@nicklan I'd expect this branch to need a number of changes before we see something like this landing on master, but overall I think we're interested in adding something to support this use case. Here's some of my thoughts on this, just looking at the code:

  • I'd want to look more closely at the writer thread that's being used here, and how that could/should be incorporated into our event loop (if at all).
  • I'd want to discuss a better abstraction than exposing a new program named "pager" for this action. As @chrisduerr eluded to above, this may be related to other features that want to make use of the contents of our grid, not just "paging" them. I think a name closer to "externalpipe" is fitting.
  • While I respect the --inherit-stdin as a cool hack to get a pager working, I'm not sure I'd want to add this in general, maybe I'm missing some greater purpose for it. Could this be done in another way without adding this flag?

https://st.suckless.org/patches/externalpipe/ gives a description as "Reading and writing st's screen through a pipe". Is writing to the grid something we'd like to support here as well? Seems a lot harder, but might enable things like #2792 and a lot of other cool external control. We generally avoid too much extensible like this out of concerns for performance, so I'd be interested to hear peoples thoughts on this. Or even if someone could point me to an interesting example of using externalpipe in ST which makes use of this.

@Deewiant
Copy link
Author

Deewiant commented Oct 4, 2019

@nixpulvis I believe you're reading too much into the description of st's patch: there's no support for modifying the terminal's screen with an external program — except, you know, in the normal ways that programs write stuff to terminals. The patch itself is very small, you can probably get the gist of it rather easily by having a look even if you're not at all familiar with st's code otherwise. I think the "reading and writing" in the description is trying to say that the screen is first read from memory and then it's written to the pipe, and that's all.

Furthermore, my intention with this feature request was to cover only the kind of functionality that the externalpipe patch for st provides. I agree with your feeling that allowing an external program to modify the grid in some (presumably alacritty-specific) way would be a lot more difficult to do. I'd like to again point out that the patch to st is rather small and hence I imagined this as being moderately easy to do for alacritty as well, with the configuration aspect possibly ending up as the trickiest part. So in my opinion that sort of extension is too far out of scope here, and if it's something you or anyone considers worth it I'd encourage writing it up as a separate issue.

@travankor
Copy link

travankor commented Dec 30, 2019

Kitty has a similar feature , ie: piping scrollback to a pager, to what was in the rejected PR #2568 (note: that Kitty has proper integration, while the PR is somewhat a hack so I'm not suggesting that the PR should be merged). This is not only useful for searching, but you can navigate through scrollback a lot faster with less/vim.

I would even say that this would be a good alternative to #775.

@kchibisov
Copy link
Member

@travankor You're not reading #2568 correctly, I think. The PR was rejected because the implementation isn't good. I think kitty doesn't spawn a new instance with pager running, right? piping scrollback to a pager make sense, but you shouldn't spawn a new alacritty instance for it.

@travankor
Copy link

travankor commented Dec 30, 2019

@kchibisov err, right I agree. I was not suggesting that the PR should have been merged, but rather that the idea behind the PR is popular enough to be supported officially (ie: see the comments 1 and 2).

However, it would be nice to know what the neccessary steps are to correctly implement this feature if someone can list them off the top of their head. Because in various issue reports people are talking about piping scrollback to editors, to searching through scrollback (ie: not with piping to an external program?), to dealing with URLs, to having a scrollbar, to having a keyboard-driven interface for controlling these features.

I'm thinking that alacritty needs some sort of scripting interface to support all of these disparate yet partially inter-related features. Alternatively, alacritty will need to support these use-cases in only one way that is elegant and is "correct".

@chrisduerr
Copy link
Member

chrisduerr commented Dec 30, 2019

@travankor See #1615 (comment).

It might not be 100% accurate anymore, but it should probably give a good enough idea of what needs to be done.

I'm thinking that alacritty needs some sort of scripting interface to support all of these disparate yet partially inter-related features.

Alacritty does not require a scripting interface and there is absolutely no desire to add one.

@kchibisov kchibisov added this to the Version 0.4.3 milestone Jan 19, 2020
@jynnantonix
Copy link

I'm not sure if this is appropriate to post here so please feel free to delete this comment if it's not.

I've rebased my changes on top of the 0.4 release here: https://github.com/jynnantonix/alacritty/tree/v0.4-dump-scrollback

I modified the command to pipe the scrollback buffer to the stdin of an arbitrary user-provided program instead of spawning a new instance of alacritty. The command can be specified in the scrollback_handler section of the config file. The default still spawns a new instance of alacritty with less because that's just what I use ;-)

I've been using it for a couple of days and it seems to be fine. If anyone reading this just wants something that works today, please feel free to give it a try and let me know if you run into any issues.

@Deewiant
Copy link
Author

I opened #3709 which addresses this the way I've always intended it, i.e. pretty much the way st does plus the option to pipe full scrollback instead of only the visible part.

(Unfortunately I then ran into #3710 and #3711 which affect my dmenu use case. There's always something...)

@pickfire
Copy link

The only thing I found useful for external pipe so far is to get urls from the text, but I believe this may be better being built-in since grepping for hyperlinks may not always work and with #922 links may even be embedded with the text so I think doing this from alacritty side would be better.

What I thought of is to let alacritty get all hyperlinks in the visible portion of the screen and pipe it somewhere, so the user can specific what to do with the links, I would probably invoke dmenu | xdg-open.

@carrascomj
Copy link
Contributor

The only thing I found useful for external pipe so far is to get urls from the text, but I believe this may be better being built-in since grepping for hyperlinks may not always work and with #922 links may even be embedded with the text so I think doing this from alacritty side would be better.

What I thought of is to let alacritty get all hyperlinks in the visible portion of the screen and pipe it somewhere, so the user can specific what to do with the links, I would probably invoke dmenu | xdg-open.

What about copying the last command (maybe computationally expensive) output lo a clipboard? Or any previous command in the session? I believe that's something that you can do with st's externalpipe.

@pickfire
Copy link

pickfire commented Oct 5, 2020

What about copying the last command (maybe computationally expensive) output lo a clipboard? Or any previous command in the session? I believe that's something that you can do with st's externalpipe.

That should be possible, but I rather it not the default for poluting the clipboard. But I don't see this quite useful, for taking url in the screen, the best way is to extract what alacritty thinks is url as of now.

@carrascomj
Copy link
Contributor

That should be possible, but I rather it not the default for poluting the clipboard.

Oh, I was not thinking of just having that as default. My use case was about, giving that we have have something like externalpipe, allowing the user to do alacritty-pipe | grep "magical pattern to take last command output" | xclip -i. It even could turn into a more complicated script with dmenu selecting which command output to copy to the clipboard.

I emphasize allowing the user because that's functionality that the user could make based on an implementation of externalpipe (which is, as stated in this issue, quite minimal) but not something that would be implemented directly in the emulator. That's something that I would find useful, for example to pipe the output of a very long and unsuccessful set of CI tests to a pager like less or, even better, to bat.

Just my opinion as an user, anyways, others may disagree.

@Axelen123
Copy link

I made my own fork because it has been quite a while since we got an update on #3709.

@Deewiant
Copy link
Author

Deewiant commented Dec 9, 2020

Yeah, apologies for that, it just hasn't been a priority for me. Chris left that interesting suggestion about parallelizing it, which I'd like to look into but it'd require more time than I've been able to spare so far.

@Axelen123
Copy link

Axelen123 commented Dec 10, 2020

Yeah, apologies for that, it just hasn't been a priority for me.

I understand. I have quite a few half finished projects myself now that I think about it.

Chris left that interesting suggestion about parallelizing it, which I'd like to look into but it'd require more time than I've been able to spare so far.

I added basic parallelization to my fork using rayon. I do have one idea which will probably use less ram but it will make it more complex and less readable.

@Deewiant
Copy link
Author

Sounds like you have something worth PRing there, maybe we can get that merged instead of #3709.

@Axelen123
Copy link

Axelen123 commented Dec 10, 2020

I agree. I will open a PR very soon.

@chrisduerr
Copy link
Member

Closing this in favor of #4550.

@Deewiant
Copy link
Author

@chrisduerr This is the feature request, did you mean to close my PR at #3709 instead?

@chrisduerr
Copy link
Member

I did indeed, thanks.

@chrisduerr chrisduerr reopened this Dec 12, 2020
@chrisduerr chrisduerr removed this from the Version 0.7.0 milestone Dec 21, 2020
@gerrywastaken
Copy link

For anybody following along. The last PR working on this was just closed: #4550 (comment).

@cshjsc
Copy link

cshjsc commented May 11, 2022

@gerrywastaken will this ever get implemented or that comment also closes this issue and we will never see such a feature.

@gerrywastaken
Copy link

gerrywastaken commented May 15, 2022

will this ever get implemented

@cshjsc I hope so, but I'm not working on it. Are you?

@methuselah-0
Copy link

FWIW: I just run emacspipe -t to open my terminal contents in emacs, using ydotool, wl-clipboard and some shellscript - #4550 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment