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

Can't alt click to move cursor #316

Closed
pdufour opened this issue Jul 20, 2016 · 24 comments
Closed

Can't alt click to move cursor #316

pdufour opened this issue Jul 20, 2016 · 24 comments
Assignees
Labels
‼️ Priority: OMG Maximum Issue needs to be seen to immediately. Like now. Please. ⚠️ 🛠 Type: Feature Request Issue or PR is a feature request/proposal for Hyper
Milestone

Comments

@pdufour
Copy link

pdufour commented Jul 20, 2016

Terminal app on mac has a nice feature where you can alt + click to move the cursor to a position. This does not work in Hyperterm.

@timneutkens
Copy link
Member

@pdufour by just looking through hyperterm issues my mind gets blown by the crazy time saving things I didn't know about iTerm and mac terminal. Thanks for sharing 😉 +1

@matheuss matheuss added the 🛠 Type: Feature Request Issue or PR is a feature request/proposal for Hyper label Jul 26, 2016
@quantizor
Copy link

This is a must-have feature.

@matheuss matheuss added the help wanted Contributions wanted towards the issue label Oct 13, 2016
@josiahwiebe
Copy link

Here's a gif for reference.

2016-10-27 08_26_56

@ppot
Copy link
Contributor

ppot commented Nov 6, 2016

The way hterm interpret it with span doesn't allow us the ability to do this now. From what I tried. I can set the cursor position but then it doesn't comply to text after it.

@matheuss matheuss added ‼️ Priority: OMG Maximum Issue needs to be seen to immediately. Like now. Please. ⚠️ and removed help wanted Contributions wanted towards the issue labels Nov 6, 2016
@matheuss matheuss added this to the v1.0.0 milestone Nov 6, 2016
@matheuss matheuss self-assigned this Nov 6, 2016
@matheuss
Copy link
Member

Copying what I wrote on #1466:

that's a thing I'd be very interested to see on Hyper. I'd be very interested on implementing it too, but unfortunately I'm incredibly busy lately 😔

@matheuss matheuss modified the milestones: 2.0.0, v1.0.0 Jan 31, 2017
@adamscybot
Copy link

Is this possible with a plugin? @ppot Can you give more detail on why it isn't doable?

Would like to have a go at this unless the API totally prevents it.

@ppot
Copy link
Contributor

ppot commented Mar 3, 2017

@adamscybot From what I tried, you can update the cursor position, but it didn't sync. You can try!

@adamscybot
Copy link

adamscybot commented Mar 3, 2017

Ah as in, the server didn't actually recognize the change? I guess you'd have to send the right control characters over the ssh channel to move the cursor, i.e. the equivalent of pressing the right or left arrow key however many keys you need to move.

@adamscybot
Copy link

adamscybot commented Mar 8, 2017

Ok, I have a working solution inside a plugin that is very very hacky. As mentioned, the lack of spans to detect click event makes this very tricky. I think proper support would need to be integrated into hyper itself. Or an improved plugin API.

Regardless, I know it will help some people so I will finish it off this week and publish (hopefully tomorrow evening, GMT).

@adamscybot
Copy link

adamscybot commented Mar 8, 2017

Ok guys here we go: https:/adamscybot/hyper-alt-click

It's a giant hack, and probably buggy, but I just needed this feature. Very experimental at this stage, hopefully room for improvement. Especially support in Vim/Nano etc.

Completely untested on everything but my mac...

@chabou
Copy link
Collaborator

chabou commented Apr 14, 2017

@adamscybot What API improvements do you need to achieve this with a less-hacky plugin?

@adamscybot
Copy link

adamscybot commented Apr 18, 2017

@chabou The main issue is that each line in the terminal exists as a text node in the DOM. This means you can not attach a click handler to an individual letter. I think there needs to be a way to pass down your own element for rendering a line in the terminal.

Im hacking around this problem by:

  • Detecting a click event on the whole line
  • Duplicating the text, but every character wrapped in a span.
  • Overlay this duplication directly over the top.
  • Add handlers to these spans.
  • Refire the click event at the same location to see which letter was clicked on.
  • Play around with offsets (also hacky, as the terminal prompt prefix thing name@machine:~$ is treated as part of the normal text) to know how many times to send the left or right control character.

Yuck!

I believe the base issue though actually exists in Google Hterm though, which seems to be responsible for the handling of the actual terminal lines. Not sure if it can easily be hacked in or if hterm would need to be replaced. I think this dependency is the limiting factor.

@chabou
Copy link
Collaborator

chabou commented Apr 18, 2017

Adding a span to every char could have a huge impact on performances.
Your hack has a great advantage: it takes place only after a alt + click event 😛

Hyper has a lot of hterm monkey patches. You can look deeper into hterm, try to add span for each chars and measure performance changes.

Hyper has already a lot of performances issues. Adding DOM complexity seems harmful.

@adamscybot
Copy link

adamscybot commented Apr 19, 2017

@chabou Yeh, I can see how that could be an issue. Perhaps one route would be to try and formalise my method in some way? If there was a way to attach a click handler on a terminal line and then to modify how lines are rendered (pass down a custom component?) on a per-line basis.

The primary issue with my hack is it blatantly ignores react by touching the DOM outside of the react ecosystem. This is outright incorrect and so the hack exists in a permanent state of instability.

@chabou
Copy link
Collaborator

chabou commented Apr 19, 2017

Hterm is itself outside react ecosystem (encapsulated in an iFrame).
And imho, it is a bad idea that Hyper hacks and exposes some hterm's low-level mechanisms. Plugin should do it by itself using onTerminal() hook to access hterm instance

@watzon
Copy link

watzon commented Aug 7, 2017

I was going to make a new issue, but this is related. In linux and OSX you can click to move the cursor in Vim and other apps. I know it's not a best practice for vim, but sometimes it's nice when I'm in a "just f*ck it" mood and don't want to deal with keyboard shortcuts.

@chabou
Copy link
Collaborator

chabou commented Aug 14, 2017

You're right.
In Terminal app, I have to alt+click to move on command line. But it works fine without alt in vim (with my config). It doesn't work in Hyper.

@chabou
Copy link
Collaborator

chabou commented Aug 14, 2017

The good news is: it works great in vim on xterm branch (replacing hterm by xterm.js)

@adamscybot
Copy link

@chabou Oooo xterm. Does this bring anything to the table in terms of per-character mouse events? Is alt+click already supported out of the box?

@chabou
Copy link
Collaborator

chabou commented Aug 15, 2017

@adamscybot col/row notion is reaaaally better handled in xterm.js. You can take a look to selection helpers https:/sourcelair/xterm.js/blob/master/src/utils/Mouse.ts.
Alt+click is not supported yet, but it is not like hterm: xterm.js is really open-source and PRs are welcome. @Tyriar will certainly be ok to have a PR to implement this alt+click directly in xterm.js because it will be great to have it in VSCode too.

@adamscybot
Copy link

Ah this is exactly what was needed. I'm guessing I could just grab the xterm instance in a hyper plugin and plug away at it. I'll take a look once its in master.

@Tyriar
Copy link
Contributor

Tyriar commented Aug 16, 2017

@adamscybot yes PRs welcome in xterm.js. If you want to contribute in this area please fork the v3 branch and make changes there to avoid merge conflict headaches. Created xtermjs/xterm.js#890 to discuss implementation 😃

Also working against the demo https:/sourcelair/xterm.js#demo will be easier than trying to pull the right version in to hyper.

@timothyis
Copy link
Contributor

Update: There's a PR in xterm for this feature xtermjs/xterm.js#896

@Tyriar
Copy link
Contributor

Tyriar commented Jan 29, 2018

And we merged it today, coming in the next version 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
‼️ Priority: OMG Maximum Issue needs to be seen to immediately. Like now. Please. ⚠️ 🛠 Type: Feature Request Issue or PR is a feature request/proposal for Hyper
Projects
None yet
Development

No branches or pull requests