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

git prompt: faster count of untracked files #3294

Closed
wants to merge 3 commits into from
Closed

git prompt: faster count of untracked files #3294

wants to merge 3 commits into from

Conversation

nomaed
Copy link
Contributor

@nomaed nomaed commented Aug 9, 2016

In a repository with 18k untracked files (by accident, node_modules not in .gitignore), counting them using count (git ls-files) was causing a considerable lag with prompt generation.

After changing it to git ls-files | wc -l, the count process is at least twice as fast. Not ideal, but it avoids caching the entire git ls-files result as a string to count the number of lines by piping the result through wc -l directly.

@nomaed
Copy link
Contributor Author

nomaed commented Aug 9, 2016

Test/demo:

$ cat count-fish
#!/usr/bin/env fish
count (git ls-files --others)

$ cat count-wc 
#!/usr/bin/env fish
git ls-files --others | wc -l

$ time -p ./count-fish
20930
real 0.18
user 0.15
sys 0.02

$ time -p ./count-fish
20930
real 0.17
user 0.14
sys 0.02

$ time -p ./count-fish
20930
real 0.19
user 0.15
sys 0.02

$ time -p ./count-wc
20930
real 0.09
user 0.04
sys 0.03

$ time -p ./count-wc
20930
real 0.06
user 0.03
sys 0.02

$ time -p ./count-wc
20930
real 0.09
user 0.04
sys 0.02

@faho
Copy link
Member

faho commented Aug 9, 2016

Minor nit: You can pipe to string trim as well (wc -l | string trim) instead of nesting command substitutions.

@faho faho added this to the next-2.x milestone Aug 9, 2016
@nomaed
Copy link
Contributor Author

nomaed commented Aug 9, 2016

@faho Good point, done.

@@ -478,7 +478,7 @@ function __fish_git_prompt_informative_status
set -l dirtystate (math (count $changedFiles) - (count (echo $changedFiles | grep "U")))
set -l invalidstate (count (echo $stagedFiles | grep "U"))
set -l stagedstate (math (count $stagedFiles) - $invalidstate)
set -l untrackedfiles (count (command git ls-files --others --exclude-standard))
set -l untrackedfiles (command git ls-files --others --exclude-standard | wc -l | string trim)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is | string trim required? The wc -l portion of that command will return a single line that the fish sub-command will turn into a single value since it strips (i.e., ignores) a trailing newline.

Copy link
Contributor Author

@nomaed nomaed Aug 10, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@faho commented about it, so I made the change
#3294 (comment)

Edit: I thought at first that you were talking about why piping instead of using a string trim with a sub-command. As @floam wrote, the reason is exactly that there are leading spaces that are saved and the string needs to be trimmed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. I forgot that there are brain-dead wc implementations (e.g., the BSD one) that left-pad the value with spaces.

@floam
Copy link
Member

floam commented Aug 10, 2016

@krader1961 The spaces will not be ignored:

> set -l foo (command git ls-files --others --exclude-standard | wc -l)
> echo $foo
     102

@krader1961
Copy link
Contributor

Thanks. Merged as dc02587.

@krader1961 krader1961 closed this Aug 10, 2016
@krader1961 krader1961 modified the milestones: fish 2.4.0, next-2.x Sep 3, 2016
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants