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

Allow different window-inherit-working-directory behavior for tabs and windows #1392

Open
adrianord opened this issue Jan 27, 2024 · 16 comments
Labels
contributor friendly A well-scoped, approachable issue for someone looking to contributor. gui GUI or app issue regardless of platform (i.e. Swift, GTK)

Comments

@adrianord
Copy link
Collaborator

I would like to have different behaviors for the window-inherit-working-directory option, either through a new option like tab-inherit-working-directory or a command that can be bound through a keybind option.

Use case would allow for news tabs to inherit the working directory while a new window would open the home directory, or vice versa.

@mitchellh mitchellh added apprt contributor friendly A well-scoped, approachable issue for someone looking to contributor. labels Jan 27, 2024
@mitchellh
Copy link
Contributor

My recommendation would be to change window-inherit-working-directory to an enum, with possible values: false (never), true (always), tab (only new tabs), window (only on new windows) or something like that. We may have to use a packed struct to allow multiple options too, so you can set tab, window, split, etc. That'd require slightly new config parsing code but it's not significant. I think that's better than a new option.

@jparise
Copy link
Collaborator

jparise commented Apr 17, 2024

I started working on this. The configuration aspect was straightforward, but I slowed down implementing the actual logic (in newConfig). My initial feeling was to do something like:

        const inherit_pwd = switch (config.@"window-inherit-working-directory") {
            .true => true,
            .false => false,
            .tab => is_tab,
            .window => !is_tab,
        };
        if (inherit_pwd) {
            if (try p.pwd(alloc)) |pwd| {
                copy.@"working-directory" = pwd;
            }
        }

... but I'm not sure how to derive an is_tab value here or thread it through as an option. It seems like the Surface type doesn't know whether it's being presented as a tab or a window, and that's handled by the various "rt" implementations without a common interface?

Or maybe we should set the new config's @"working-directory" value elsewhere?

I'm happy to finish implementing this with a little more guidance on what other bits should change.

@mitchellh
Copy link
Contributor

It seems like the Surface type doesn't know whether it's being presented as a tab or a window, and that's handled by the various "rt" implementations without a common interface?

That's right.

I think that the answer is that newConfig needs to take a surface kind enum. Looking quickly at the code, newConfig is only triggered by apprts via more specific callbacks like newTab, newWindow, etc. so it doesn't seem difficult to make that information available to it.

I'd try to limit the core's actual knowledge about this and keep it within the apprts.

I'm not 100% sure that'll work, its hard to say without actually trying to go through it myself, but that's what I'd try first.

@clason
Copy link
Collaborator

clason commented May 19, 2024

We may have to use a packed struct to allow multiple options too, so you can set tab, window, split, etc.

I would second that suggestion; personally, I would like for new tabs to "start fresh", while a split is more like a second "view" on the current context (i.e., should inherit cwd).

@d-strygwyr
Copy link

will released soon?

@jparise
Copy link
Collaborator

jparise commented Dec 31, 2024

@mitchellh and I have been talking through an implementation approach in a few places (#1694 and Discord). The most recent thought was to move toward a "surface kind-aware" configuration dimension and then use that to allow per-kind conditional configuration of the window-inherit-working-directory option. For example, using pseudo syntax: window-inherit-working-directory[window] = false


Here are my latest thoughts after sitting with that (conditional config) idea for a bit:

  1. The idea of telling a Surface its kind would be initially limited to its initialization (to be used as a config hint). Maintaining its kind, while maybe useful in other ways, would require updating it whenever the surface is moved around by the apprt via some new hooks/callbacks. This is also relevant to the "state restoration" path, which needs to know how to recreate a surface.
  2. The "working directory" is currently an initialization option that's also passed into the Surface initialization path (at least in the embedded apprt case). We may need to rework that a bit so that we can use the conditional config to inform whether or not we use the inherited value.

Maybe the theme here is that this is all going to be about a surface's "initial" configuration, so I think there shouldn't be an expectation that we'd update e.g. the theme if a surface changes from a split to a window, nor am I sure that would be particularly useful even if we could support it. Maybe? This could become its own discussion.

So I think I still have a preference for the simpler "tagged union" approach (e.g. window-inherit-working-directory = tab,split), but that's probably because the amount of plumbing to stand up this new configuration dimension feels like a lot of work for this small option. Are there many other configuration options that make sense to configure on a per-surface-kind-basis?

@sethvargo
Copy link
Collaborator

(If this isn't helpful, feel free to disregard...) My brain doesn't really consider "splits" or "tabs" to be "windows":

  • Windows:
    window

  • Tabs:
    tab

  • Splits (example is horizontal, but same for vertical):
    split

This would be my proposal:

  • Introduce a new config surface-working-directory
    • surface-working-directory has a default that applies to all new surfaces (windows, tabs, splits)
    • surface-working-directory[window], surface-working-directory[tab], surface-working-directory[split] configure the working directory for new windows, tabs, and splits respectively (overriding the default)

I think that's pretty similar to the original proposal, but it actually introduces the word "surface" instead of overloading "window", and it provides a default value for simple configs.

@mitchellh
Copy link
Contributor

To add to the "surface kind aware" note, we should also pipe through metadata about the kind of terminal we're looking at, because there is a significant amount of requests for macOS quick terminal specific configuration.

@ggregoire
Copy link

I would second that suggestion; personally, I would like for new tabs to "start fresh", while a split is more like a second "view" on the current context (i.e., should inherit cwd).

Yes that's exactly the configuration I'm using in iterm2.

I want a split to inherit the working directory of its tab and a new tab/window to open on ~.

For example If I have a project running in a tab and I want to see its git log, I just split the tab and do git log (same working directory). But if I want to do anything else not related to this project (different working directory), I open a new tab.

My proposal was to have three separate settings (grouped together in the documentation):

  • window-inherit-working-directory
  • tab-inherit-working-directory
  • split-inherit-working-directory

And if tab-inherit-working-directory and split-inherit-working-directory are not set, then tabs and splits inherits their config from window-inherit-working-directory (as of today).

@0az
Copy link

0az commented Jan 9, 2025

There's one more configuration axis that hasn't been brought up: the ability for separate keybinds that retain / don't retain the current working directory.

As an example, in Kitty, I bind cmd-enter to "new split, retain CWD", which is useful for one-off commands when I have an editor taking up an entire maximized tab. In contrast, cmd-alt-enter will just use $HOME, which is useful for a different type of one-off command.

Sometimes I would rather tab back/forth instead of using splits, such as when I'm using a laptop screen instead of a desktop monitor. In that case, cmd-alt-t will create a new tab in the current CWD, versus cmd-t which resets to $HOME.

Prior art: Kitty's new_tab/new_window (split) actions have _with_cwd variants

https://sw.kovidgoyal.net/kitty/actions/#action-new_tab
https://sw.kovidgoyal.net/kitty/actions/#action-new_tab_with_cwd
https://sw.kovidgoyal.net/kitty/actions/#action-new_window
https://sw.kovidgoyal.net/kitty/actions/#action-new_window_with_cwd

@bsushmith
Copy link

bsushmith commented Jan 16, 2025

I am facing a peculiar issue and I think it's related to the issue in this thread. Please correct me if it's not.

Whenever I do an SSH to a machine in a given tab on ghostty and then do a split using ⌘ + D, the split opens with the present working directory as the ~/.ssh folder - irrespective of where I run the ssh command in the initial tab. The new tab should either have the same cwd or a specific cwd, but not .ssh folder. This seems like an unintended behaviour.

Screenshot 1 is when I do a ssh from a specific folder(doesn't matter if its home folder), the split opened with cwd as ~/.ssh

Screenshot 1:
Image

Screenshot 2 is the normal behaviour.

Screenshot 2:
Image

My Ghostty config:

macos-option-as-alt = left

@0az
Copy link

0az commented Feb 9, 2025

So, folks that want separate inherit-working-directory values for windows/tabs/splits: would you be satisfied if you could do this at the keybind/action level?

I think this is a more powerful control than simply adding a setting for each, and it sounds like this may avoid the issue of context that ended up blocking PR-1694.

That this would also be personally convenient is a nice bonus. :)

@SamirTalwar
Copy link

Personally, I don’t need them to be separate keybindings (as I always want the same behaviour for splits/tabs/windows), but it would not be an issue for me if they were separate, as I’d just bind the correct one for my purposes.

So no downsides, and others (including you devs) benefit. I think it’s a very good solution.

(In case it’s relevant: I want splits to preserve PWD, and tabs and windows to forget it.)

@mfilej
Copy link

mfilej commented Feb 9, 2025

So, folks that want separate inherit-working-directory values for windows/tabs/splits: would you be satisfied if you could do this at the keybind/action level?

Yes! This is what I had with Kitty and it worked well.

@joeriddles
Copy link

So, folks that want separate inherit-working-directory values for windows/tabs/splits: would you be satisfied if you could do this at the keybind/action level?

I think this would work for me. If I can configure a keyboard shortcut to open a new tab without the same CWD but configure window splitting to inherit the CWD, that would be awesome.

@helgelol
Copy link

helgelol commented Feb 9, 2025

So, folks that want separate inherit-working-directory values for windows/tabs/splits: would you be satisfied if you could do this at the keybind/action level?

I think this would work for me. If I can configure a keyboard shortcut to open a new tab without the same CWD but configure window splitting to inherit the CWD, that would be awesome.

Exactly this, i want my split to inherit, and new tab to start in home folder.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor friendly A well-scoped, approachable issue for someone looking to contributor. gui GUI or app issue regardless of platform (i.e. Swift, GTK)
Projects
None yet
Development

Successfully merging a pull request may close this issue.