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

Mark untracked as dirty; fix dirty marker when no HEAD ref #286

Merged
merged 5 commits into from
Aug 17, 2023

Conversation

spthm
Copy link
Contributor

@spthm spthm commented Aug 12, 2021

Summary of changes:

  • Add the dirty marker when untracked files are present
  • Fix behaviour of dirty marker when there is no HEAD
  • Test _pure_prompt_git_dirty when HEAD does and does not exist

Currently, _pure_prompt_git_dirty does not add the dirty maker for untracked files. The implementation here is as it's done in git/contrib/completion/git-prompt.sh.

When I went to go add a test for this feature, I discovered it was already there - and passing! This is because the git diff-index [...] HEAD -- used gives a non-zero exit code when HEAD is not present. All repos without a HEAD are therefore marked as dirty. During testing, the repo does not have a HEAD.

This PR fixes that, falling back to a git diff --staged when HEAD is not present. It also ~duplicates the existing tests to check the dirty marker works as expected immediately after git init (i.e. no HEAD) and after an initial commit (i.e. with a HEAD).

To (maybe) clarify things a bit: with the tests in this PR, both _pure_prompt_git_dirty: empty repo is not marked as dirty and _pure_prompt_git_dirty: untracked files mark git repo as dirty fail on master.

Wasn't sure if this should be behind a new feature flag (e.g. pure_untracked_is_dirty). It is documented/expected behaviour, but this fix might slow down the prompt in large repos (c.f. #189). FWIW I did a quick check with the full cpython source and on my machine the time is comparable to the existing commands,

spthm@local:~/cpython main $ touch file.txt
spthm@local:~/cpython main $ time git ls-files --others --exclude-standard --directory --no-empty-directory --error-unmatch -- ':/*'
file.txt
git ls-files --others --exclude-standard --directory --no-empty-directory  --  0.02s user 0.03s system 84% cpu 0.052 total
spthm@local:~/cpython main $ time git diff-index --ignore-submodules --cached --quiet HEAD -- >/dev/null 2>&1 
git diff-index --ignore-submodules --cached --quiet HEAD -- > /dev/null 2>&1  0.01s user 0.02s system 54% cpu 0.055 total

@edouard-lopez
Copy link
Member

edouard-lopez commented May 19, 2022

Hello @spthm,
Thank you for your patience, I'm resuming my work on pure. I will first focus on fixing the tests, before merging the existing request that touch to the codebase, to ensure we have no regression.

@edouard-lopez
Copy link
Member

Hello @spthm,
Thanks for spotting this and taking time to submit a PR.
After rebasing your MR on 3672fb8 and got 3 failing tests. Is it expected?

Your MR fix/finish an incomplete behavior on the git feature, the feature flag, sound superfluous. If we get feedback that it is negatively impacting the speed of the prompt, then we can add a feature flag.

Test output

I don't have the permissions to push force the rebase on your repo, so here is the log:

ok 139 _pure_prompt_git_dirty: empty repo is not marked as dirty                                                                                                                                   
…
ok 142 _pure_prompt_git_dirty: clean is not marked as dirty                                                                                                                                        
test: Expected a combining operator like '-a' at index 2                                                                                                                                           
On branch master nothing to commit, working tree clean  = *                                                                                                                                        
                 ^                                                                                                                                                                                 
Standard input (line 10):                                                                                                                                                                          
                if test $argv                                                                                                                                                                      
                   ^                                                                             
in function '@test' with arguments '_pure_prompt_git_dirty:\ untracked\ files\ mark\ git\ repo\ as\ dirty On\ branch\ master nothing\ to\ commit,\ working\ tree\ clean "" = \*'
        called on line 54 of file /tmp/.pure/tests/_pure_prompt_git_dirty.test.fish              
not ok 143 _pure_prompt_git_dirty: untracked files mark git repo as dirty                        
  ---                                                                                            
    operator: nothing to commit, working tree clean                         
    expected: ''                                                                                 
    actual: 'On branch master'                                                                   
    at: /tmp/.pure/tests/_pure_prompt_git_dirty.test.fish:54                                     
  ...                                                                                            
test: Expected a combining operator like '-a' at index 2                                                                                                                                           
On branch master nothing to commit, working tree clean  = *              
                 ^                              
Standard input (line 10):                                                                        
                if test $argv                                                                    
                   ^                                                                                                                                                                               
in function '@test' with arguments '_pure_prompt_git_dirty:\ staged\ files\ mark\ git\ repo\ as\ dirty On\ branch\ master nothing\ to\ commit,\ working\ tree\ clean "" = \*'
        called on line 65 of file /tmp/.pure/tests/_pure_prompt_git_dirty.test.fish
not ok 144 _pure_prompt_git_dirty: staged files mark git repo as dirty
  ---                                                                                            
    operator: nothing to commit, working tree clean           
    expected: ''                                                                                 
    actual: 'On branch master'                                                                   
    at: /tmp/.pure/tests/_pure_prompt_git_dirty.test.fish:65
  ...
not ok 145 _pure_prompt_git_dirty: symbol is colorized
  ---
    operator: =
    expected: \e\[90m\*
    actual: ''
    at: /tmp/.pure/tests/_pure_prompt_git_dirty.test.fish:77
  ...

@spthm
Copy link
Contributor Author

spthm commented Jul 15, 2023

@edouard-lopez sorry it took so long to respond.

I had misunderstood how the git dirty tests work, thinking each was isolated/running from a fresh directory.

I have now fixed the tests so they take account of changes made by other tests. All tests are passing for me locally,

$ make build-pure-on FISH_VERSION=3.3.1
...
$ make test-pure-on FISH_VERSION=3.3.1
...
# /tmp/.pure/tests/_pure_prompt_git_dirty.test.fish
ok 140 _pure_prompt_git_dirty: empty repo is not marked as dirty
ok 141 _pure_prompt_git_dirty: untracked files in empty repo marked as dirty
ok 142 _pure_prompt_git_dirty: staged files in empty repo marked as dirty
ok 143 _pure_prompt_git_dirty: clean is not marked as dirty
ok 144 _pure_prompt_git_dirty: untracked files mark git repo as dirty
ok 145 _pure_prompt_git_dirty: staged files mark git repo as dirty
ok 146 _pure_prompt_git_dirty: symbol is colorized
...
1..230
# pass 230
# ok

spthm and others added 5 commits August 17, 2023 17:59
This now passes the git dirty prompt tests.
These tests run in sequence in the same directory, they are not
isolated. Tests need to take account of changes to the directory state
made by previous tests.
@edouard-lopez
Copy link
Member

That's weird, test should have been isolated. I'm adding a call to before_each/after_each around each test to clean up. see ddc3a44

@edouard-lopez edouard-lopez merged commit f561ef0 into pure-fish:master Aug 17, 2023
8 checks passed
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.

2 participants