Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

shell_integration: Report current working directory as OSC 7 #6481

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.

Sign up for GitHub

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

Jump to bottom
Merged
fdncred merged 1 commit into from
Sep 13, 2022
Merged

shell_integration: Report current working directory as OSC 7 #6481

fdncred merged 1 commit into from
Sep 13, 2022

Conversation

@valpackett
Copy link
Contributor

@valpackett valpackett commented Sep 4, 2022 •
edited
Loading

Description

OSC 7 is a de-facto standard supported by many terminals, originally added to macOS Terminal.app, now also supported by libvte (GNOME), Konsole (KDE), WezTerm, and more.

Tested with WezTerm on unix only, not Windows… BTW, Windows users might be interested in also adding OSC 9;9, but I'm not testing on Windows. (looks like some code collision danger there though: WezTerm docs say OSC 9 is also "iTerm2 Show System Notification"!)

To test on WezTerm: navigate to a directory, open a new tab, check that it opens in the same directory.

Tests

Make sure you've done the following:

  • 🤷 Add tests that cover your changes, either in the command examples, the crate/tests folder, or in the /tests folder.
  • 🤷 Try to think about corner cases and various ways how your changes could break. Cover them with tests.
  • If adding tests is not possible, please document in the PR body a minimal example with steps on how to reproduce so one can verify your change works.

Make sure you've run and fixed any issues with these commands:

  • cargo fmt --all -- --check to check standard code formatting (cargo fmt --all applies these changes)
  • cargo clippy --workspace --features=extra -- -D warnings -D clippy::unwrap_used -A clippy::needless_collect to check that you're using the standard code style
  • cargo test --workspace --features=extra to check that all the tests pass

@valpackett
shell_integration: Report current working directory as OSC 7
ce44b22 
This is a de-facto standard supported by many terminals, originally
added to macOS Terminal.app, now also supported by VTE (GNOME),
Konsole (KDE), WezTerm, and more.
@fdncred
Copy link
Contributor

fdncred commented Sep 5, 2022 •
edited
Loading

With the current shell_integration enabled, my WezTerm on MacOS already starts in the same directory my first tab was in without this PR.

These are the typical terminals that I like to test on to ensure ansi escapes work on. I think all our sequences are compatible on all these at the moment.

MacOS
Terminal
iTerm2
WezTerm
Alacritty
Kitty

Windows
Windows Terminal
WezTerm
Alacritty

Linux
Gnome-Terminal
WezTerm
Alacritty
Kitty

@fdncred fdncred marked this pull request as draft September 5, 2022 01:40
@valpackett
Copy link
Contributor Author

valpackett commented Sep 5, 2022

starts in the same directory my first tab was in

first and was sound like the key words here? :) with OSC 7 new tabs will open in the current directory of the currently focused pane.

GNOME Terminal does the same thing as WezTerm:

osc7gnome.mp4

Also (unless something changed in the N years since I've touched it) I think in macOS Terminal the path should show up in the window title bar as a draggable / right-clickable directory thing like in Finder etc.

Visual Studio Code and iTerm2 (if a checkbox is ticked?) should also support it.

@fdncred
Copy link
Contributor

fdncred commented Sep 7, 2022 •
edited
Loading

I'm not opposed to the PR but let's check some terminals to see how they work. I'll test some but if someone wants to let me know the results I'll update this table too.

  • Works means it works, duh
  • NoChange means it doesn't work because the terminal doesn't support it, but it also doesn't cause any noticeable issue.
  • Error like seeing ansi escapes or something else
OS Terminal Works, NoChange, Error Notes
MacOS Terminal Works
MacOS iTerm2 Works
MacOS WezTerm Works
MacOS Alacritty NoChange
MacOS Kitty NoChange
Windows Windows Terminal NoChange I don't think it's supported yet
Windows WezTerm Works
Windows Alacritty NoChange I'm not sure if alacritty supports it. i can launch another window but it always starts in the same initial folder
Linux Gnome-Terminal Works
Linux WezTerm Works
Linux Alacritty NoChange
Linux Kitty NoChange

@fdncred fdncred marked this pull request as ready for review September 13, 2022 12:36
@fdncred
Copy link
Contributor

fdncred commented Sep 13, 2022

I've tested all I could. Let's land it now.

@fdncred fdncred merged commit df6a7b6 into nushell:main Sep 13, 2022
@valpackett valpackett deleted the osc7 branch September 18, 2022 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@valpackett @fdncred
Morty Proxy This is a proxified and sanitized view of the page, visit original site.