-
Notifications
You must be signed in to change notification settings - Fork 17
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
Add OSC support, add OSC 8 (anchor) support to the public api, add a … #85
Conversation
…simple implementation in VirtualTerminal, and use in 'input' example. Fixes varabyte#84.
…rCodeConverter. This allows VirtualTerminal to process sequences produced outside of kotter, e.g. by jansi.
sorry, bit of a github noob. That second commit is not part of this PR (but merge it if you like). Should have created a branch first. |
6d404e7
to
67bc66b
Compare
Oh no I didn't notice this until just now. 😱 Sorry to sit on your PR for so long. I will take a look this week |
So I finally got a chance to look into this at last. I just wanted to thank you again, so much, for sharing the code you had. I've finally gotten on top of things so I could finally start porting it over (the approach is mostly the same although I tweaked a few things here and there due to my familiarity with some of the nuances of the codebase :). Feel free to see what I've got so far here if you're curious to see how it compares: main...feature+osclink At this point I've hooked up everything except for the virtual terminal, and I want to add tests for it too (Kotter now has support for writing tests! It didn't went you first wrote this PR). I'll confirm my version works on Mac later, but it does work on Windows. However, it doesn't work for either of the Linux consoles I tried (even though one is Gnome terminal and it should have been supported on that one for years). I'm a little nervous about releasing a feature that you might think works just fine on your terminal but then it won't work for a bunch of users on Linux. Maybe it's OK as long as I just document the limitation. I need to sleep on it and another day to play around with stuff. (I'm deciding whether I want to slip this into 1.0.0 or not) Once I have this working, I'll close this PR as rejected, since I went in a slightly different direction in some ways from your approach, but I will definitely give you credit for this feature in the release notes and commit message. |
And I just checked on a default Ubuntu install, and it works there. So it might just be my own local flavor of Linux that is busted. I'm pretty convinced at this point I want to get this into 1.0.0 in that case. |
BTW, just want to emphasize that the code diff I shared is a WIP. So it may change as you keep refreshing it. I definitely appreciate how thorough you were with the feature. I've actually decided in 1.0 to only allow a very constrained form of declaring links (e.g. I opened #91 to track this decision. |
Closing this pull request since the feature is now in. Looking back over my old code, I had a lot of warts, e.g. in the Ansi section. Hard to remember what I was thinking! Thanks for navigating through all that stuff. |
…simple implementation in VirtualTerminal, and use in 'input' example. Fixes #84.
Hope this is useful for you.
Running in iTerm...
Also see the definitive reference https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda