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

Signal CWD change to terminal via OSC-7 #1148

Merged
merged 3 commits into from
Aug 30, 2021
Merged

Signal CWD change to terminal via OSC-7 #1148

merged 3 commits into from
Aug 30, 2021

Conversation

dnkl
Copy link
Contributor

@dnkl dnkl commented Aug 29, 2021

The path still needs to be URL encoded, but this shows the gist of it. Tested in foot only so far. Will hopefully be able to try a few more tomorrow.

TODO

  • URL encode path
  • Test more terminals

Closes #1147

src/nnn.c Outdated Show resolved Hide resolved
src/nnn.c Outdated Show resolved Hide resolved
@jarun jarun marked this pull request as ready for review August 30, 2021 14:20
@jarun jarun merged commit 0a51874 into jarun:master Aug 30, 2021
@jarun
Copy link
Owner

jarun commented Aug 30, 2021

I have merged this PR. I will force push the appropriate changes as well.

Please let me know if you see any issues during your tests.

@jarun jarun changed the title [draft] signal CWD change to terminal via OSC-7 Signal CWD change to terminal via OSC-7 Aug 30, 2021
jarun pushed a commit that referenced this pull request Aug 30, 2021
* Signal CWD change to terminal via OSC-7

Closes #1147

* Make OSC-7 emission gated by NOX11

* Use newpath variable in gethostname()

Use dynamic memory for hostname
@jarun
Copy link
Owner

jarun commented Aug 30, 2021

Please confirm commit 0556ac1 works fine in your tests.

@dnkl
Copy link
Contributor Author

dnkl commented Aug 30, 2021

That commit works fine in both foot, and in gnome-terminal.

Technically, the path needs to be URL encoded. In reality, not doing will work in probably 99% of all cases. I tested switching to a directory named åöä&>, and it worked in gnome-terminal as well as in foot.

I also tested a directory that included a literal ESC, and that did not work. That's expected, since an ESC will typically reset the VT state machine.

It would be fairly simple to implement a URL encoder that allows plain ASCII but %NN encodes all other characters. Such an encoder would be overly eager, compared to what's actually necessary, but would produce "safe" output.

On the other hand, if we want to keep things simple, then let's skip URL encoding. The worst thing that happens is the new terminal doesn't spawn in the correct directory.

@jarun
Copy link
Owner

jarun commented Aug 30, 2021

I tested switching to a directory named åöä&>, and it worked in gnome-terminal as well as in foot.

That's because by default nnn is linked to ncursesw (with wide char support) so wide chars work as they should under the hood.

@jarun
Copy link
Owner

jarun commented Aug 30, 2021

BTW, given the restrictions in nnn, no one would agree to have an encoder for a single-line printf() that in fact, creates issues in certain workflows. Practically speaking, there's no ROI.

@dnkl
Copy link
Contributor Author

dnkl commented Aug 30, 2021

Then it's all good from my side.

@jarun
Copy link
Owner

jarun commented Aug 30, 2021

👍

@github-actions github-actions bot locked and limited conversation to collaborators Sep 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Emit OSC-7 escape when changing directory
2 participants