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

Optimise vi detection logic and use internal macros and commands as much as possible #321

Closed
wants to merge 4 commits into from

Conversation

keks24
Copy link

@keks24 keks24 commented Oct 6, 2022

Resolves #300 and should also resolve #295.

This pull-request uses:

  • The internal macro#{version} to determine the current tmux version1
  • Else clauses for if-shell2
  • display-message -p to output the version in bash3
  • printf, sort and head to check, if the current tmux version is higher, than 3.0. The last two commands come from the package coreutils, which has more portability, than bc.
  • The vi detection logic uses a pseudo substring match by substituting g?(view|n?vim?x?)(diff)?$ from #{pane_current_command}.4

Also everything is refactored to use internal macros and commands as much as possible to cause less load and invoking less subshells.

I was not sure about the vim-part, but left a comment there with a snippet, which could be used.

I adapted everything by logic and tested only certain parts of the plugin.

-Ramon

1
See CHANGES FROM 2.3 TO 2.4, 20 April 2017
https://github.com/tmux/tmux/blob/master/CHANGES

2
See CHANGES FROM 1.5 TO 1.6, 23 January 2012
https://github.com/tmux/tmux/blob/master/CHANGES

3
See CHANGES FROM 1.1 TO 1.2, 10 March 2010
https://github.com/tmux/tmux/blob/master/CHANGES

4
See CHANGES FROM 2.1 TO 2.2, 10 April 2016
https://github.com/tmux/tmux/blob/master/CHANGES

@keks24 keks24 changed the title Optimise vi detection logic Optimise vi detection logic and use internal macros and commands as much as possible Oct 6, 2022
@christoomey
Copy link
Owner

Hi @keks24, I certainly appreciate the attempts to clean things up, but unfortunately I believe this would be a regression around the handling of subprocesses (e.g. a git alias / subcommand that spawns vim). The use of ps was introduced a while back to handle the subprocess consideration and I'd not want to break that functionality.

More generally, while I definitely appreciate the attempts to clean things up and make things more efficient, I'm unlikely to merge a sweeping change like this to the core Vim detection logic. My primary goal these days is to keep things stable and a change like this would require a ton of testing (in addition to the more pointed concerns outlined above).

With all that in mind, I'm going to close this now.

@christoomey christoomey closed this Oct 8, 2022
@keks24
Copy link
Author

keks24 commented Oct 9, 2022

I see!

At least it is documented here and others might still have a use of it. :)

-Keks

@christoomey
Copy link
Owner

Absolutely! And the nice thing is that this part is easily swappable by folks so they can grab it if they specifically looking for the efficiency gains 🙂

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.

is_vim does not detect vim running in a subshell
2 participants