Show branch name when cmder.status=false without expensive status ops #2549
Conversation
chrisant996
left a comment
There was a problem hiding this comment.
Overall it looks cool. If I'm reading it right, then the way that cmderGitStatusOptIn now can be a bool or a string seems problematic (see specific comments for details).
A couple alternatives could be:
- Consider updating the usage of
cmderGitStatusOptInin the clink-completions repo as well. - Consider making a new config setting instead of modifying an existing one.
Or @daxgames may have something else in mind.
vendor/clink.lua
Outdated
| -- Has branch => therefore it is a git folder, now figure out status | ||
| local gitStatus = get_git_status() | ||
| local gitConflict = get_git_conflict() | ||
| if cmderGitStatusOptIn ~= 'branchonly' then |
There was a problem hiding this comment.
The vladimir-kotikov/clink-completions repo has a dependency on cmderGitStatusOptIn being a true/false value. The contents of that repo are copied into the %CMDER_ROOT%\vendor\clink-completions directory.
This new setting won't behave as intended in certain repos, and when it's used it will bring back the slow behavior #2484 that was fixed by 6027ac3.
There was a problem hiding this comment.
Thanks, wasn't aware of this.
Would the following seem OK instead?
local statusSetting = get_git_status_setting()
cmderGitStatusOptIn = statusSetting == true
...
if statusSetting ~= 'branchonly' then
I'll be unavailable for the next few days but can come back and iterate on this next week.
There was a problem hiding this comment.
Maybe a more important question is... do we want clink-completions's filter to still run? The goal of this PR was to avoid expensive operations, and the clink-completions filter appears to only be doing fast operations that aren't affected by repo size.
In which case I could do cmderGitStatusOptIn = statusSetting ~= false so that 'branchonly' keeps the clink-completions filter on.
There was a problem hiding this comment.
I am not aware of anyone ever complaining we showed the Git branch in the prompt. The only complaint has ALWAYS been the prompt can be slow in a large Git repo.
My thought was that *status = false changes from 'no status and no branch' to 'no status with branch' so we scrap the cmdstatus = branchonly setting.
Currently Cmder's Git Opt Out functionality has feature parity across Cmd, Powershell, and Bash shells this PR only affects git status opt out in Cmd shells so we should also:
- Modify
%cmder_root%\vendor\git-prompt.sh to just display the branch for Bash shells ifstatus=falseorbashstatus=false - Modify
%cmder_root%\vendor\psmodules\Cmder.ps1to just display the branch for Powershell shells ifstatus=falseorpsstatus=false`- We do not have much control of how Powershell prompt is displayed because we use a function from
Posh-Gitso this will be more difficult.
- We do not have much control of how Powershell prompt is displayed because we use a function from
This would maintain current compatibility with bash and powershell shells for these settings.
Lastly disabling git status checks in Cmder whether the branch is displayed or not the clink-completions git-prompt filter should not run either.
There was a problem hiding this comment.
the clink-completions filter appears to only be doing fast operations that aren't affected by repo size.
It only seems that way because most of us don't use repos large enough to reproduce the problem. I recommend reading #2484, and especially this comment and this comment with the detailed explanation.
There was a problem hiding this comment.
This all sounds good to me, so I've:
- Removed the "branchonly" concept
- Made
cmder.*status=falseonly prevent the status operations, not printing branch name for cmd - Added new simple branch name logic for bash and powershell to mimic the behavior of cmd in the
status=falsecase
There was a problem hiding this comment.
(And I'm no longer touching anything related to clink-completions so that is still off when status=false)
There was a problem hiding this comment.
The .lua part looks good to me now. I have no familiarity with the affected .sh or .ps1 scripts or how Cmder integrates with those shells, though.
There was a problem hiding this comment.
Will test it out later. Thanks
|
Does not seem to work for me:
EDIT: Powershell and bash(After copying the edited git-prompt.sh from your change to |
|
@ian-craig I opened a PR to your branch that fixes the
|
|
Thanks @daxgames, I've merged your changes and am happy with the switch to white. Unrelated side note... Is there supposed to be no space before the branch name in cmd? It looks like there used to be a space in older versions for cmd, and still is for powershell and bash. I thought I'd messed something up in my testing but it looks like your screenshots show this too. |
Fixes #2548
Adds support for skipping status check (git status and git diff) when generating the prompt, while still showing the branch name.
Adds a simple mode for powershell and bash which get the branch name by just reading
.git/HEADwhich should not slow down with large repos.I decided to change the color to gray when
status=falseto indicate that the status is unknown. This would avoid misleading people into thinking they are "clean" if they apply the config only to specific repos.