Skip to content

Change switchTabsWithPanelJumpKeys default to true #4516

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

Closed

Conversation

alcpereira
Copy link

  • PR Description

As mentioned in #4288 comment (here), the new configuration switchTabsWithPanelJumpKeys breaks the previous default behaviour leading to various issues opened related to this new default.
This PR changes the default to the regular behaviour while maintaining the feature for those who want to opt-out of this behaviour.

  • Please check if the PR fulfills these requirements
  • Cheatsheets are up-to-date (run go generate ./...)
  • Code has been formatted (see here)
  • Tests have been added/updated (see here for the integration test guide)
  • Text is internationalised (see here)
  • If a new UserConfig entry was added, make sure it can be hot-reloaded (see here)
  • Docs have been updated if necessary
  • You've read through your own file changes for silly mistakes etc

@stefanhaller
Copy link
Collaborator

I would tend to be in favor of this change (although actually I'm on the fence). However, @jesseduffield was very clear about not wanting to change the default, in the comment right below the one that you linked to in the PR description.

Also, when you say "breaks the previous default behaviour", this could mean two things: break the behavior from before we even introduced the tab switching, or later from when we introduced the config and turned it off by default. It's the former that we got massive complaints about (see #3995), that's why we turned it off by default.

@alcpereira
Copy link
Author

I would tend to be in favor of this change (although actually I'm on the fence). However, @jesseduffield was very clear about not wanting to change the default, in the comment right below the one that you linked to in the PR description.

Also, when you say "breaks the previous default behaviour", this could mean two things: break the behavior from before we even introduced the tab switching, or later from when we introduced the config and turned it off by default. It's the former that we got massive complaints about (see #3995), that's why we turned it off by default.

Thanks for pointing it out, I was not sure sure to grasp 100% @jesseduffield point of view, but I've found another comment from him explicitly saying that false should be the default (here).

I started using Lazygit after this feature and my muscle memory was built with 22 instead of 2], so I assumed it was the default behaviour for a long time, but now I realised it was not the case.

Should I close the PR? Or wait for Jesse to confirm?

@stefanhaller
Copy link
Collaborator

Should I close the PR? Or wait for Jesse to confirm?

I'm fine with leaving it open. Who knows, maybe we'll change our mind at some point if many more people ask why the tab switching is broken. 😄

@alcpereira alcpereira force-pushed the fix/switch-tabs-default branch from b2afec0 to fb3d99a Compare April 29, 2025 15:01
@jesseduffield
Copy link
Owner

jesseduffield commented May 2, 2025

Indeed: I do not want this on by default. There is a lot of value in having the jump keys behave deterministically regardless of the UI state. That is, no matter where I am, I can press '2' and end up in the files panel. If you have to take note of where you are, it diminishes the value of the jump key, given that we already have a means to get to the files panel in a way that does depend on the UI state (using tab / arrow keys / square brackets).

Now, although having jump keys also switch tabs is less work than using arrow keys and square brackets, the fact that it's so rare that you actually need to use a non-default tab does make it very annoying for the people who just want deterministic jump keys.

So I very strongly think that it should remain disabled by default. Unfortunately we're in an awkward spot right now where there's a cohort of people who are used to the old default and a cohort who are used to the new default, but I think that the old default was actually the right one.

EDIT: and by old I mean original

@alcpereira
Copy link
Author

Closing the PR as we have a clear answer :)

@alcpereira alcpereira closed this May 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants