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

Remove dependence on git config --get-color (implement Git coloring internally) #278

Closed
scottchiefbaker opened this issue Jan 3, 2018 · 8 comments

Comments

@scottchiefbaker
Copy link
Contributor

Right now d-s-f shells out to Git 5+ times to get color configuration information. This is needed to translate Git color strings into ANSI escape sequence. This process slows down startup time.

Ideally we can read the entire Git config once with git config --list and process the entries one by one. We'll need a list of the Git supported strings and what ANSI codes they should map too.

Example strings: "red bold", "green bold 22", "red reverse", "yellow"

@scottchiefbaker
Copy link
Contributor Author

@peff could you provide a starting point on the Git side about where this information is stored? Or a list of what strings Git accepts?

@peff
Copy link

peff commented Jan 3, 2018

The interesting entry point is https://github.com/gitster/git/blob/4a4ac8367882698ac607169f65964c344f74d14c/color.c#L202, which in turn relies on parse_color and parse_attr. t's pretty straightforward for porting, I think.

I do still think you'd be better served by a git config --stdin feature. I had somebody working on it during a hackfest in November, but unfortunately they seem to have stalled halfway through. 😢

@scottchiefbaker
Copy link
Contributor Author

Can you elaborate on git config --stdin?

@peff
Copy link

peff commented Jan 4, 2018

I mean that ideally you could do something like:

git config --stdin <<\EOF
key=color.diff-highlight.oldnormal
type=color

key=color.diffhighlight.oldhighlight
type=color
...etc...
EOF

and get output something like:

key=color.diffhighlight.oldnormal
value=[some ansi codes]
...etc...

all with a single invocation of git config. But that feature doesn't exist yet.

@scottchiefbaker
Copy link
Contributor Author

scottchiefbaker commented May 17, 2021

At startup diff-so-fancy shells out to git --config five times to read color data from the config file. This is slowest part of startup and can make d-s-f feel sluggish. It has been a personal pet peeve of mine for a while, and I would like to speed up the startup time.

I ran d-s-f five times with the parsing code in place and got an average of 81 ms to show a simple diff. Then I hardcoded color_config() to always return 4 and re-ran the tests and I got an average of 46 ms to show the same diff. In other words, shelling out to check colors almost doubles startup time.

All tests were run with a simple:

time git diff

@scottchiefbaker
Copy link
Contributor Author

@peff did git config --stdin ever get implemented?

@peff
Copy link

peff commented May 19, 2021

@scottchiefbaker Sorry, no, nobody ever implemented it (I still think it's a good idea).

One hack somebody proposed recently upstream (for send-email, which reads a lot of config) is to use git config --list --name-only to find out the set of available config, and then call individual git config commands to retrieve the interpreted values of ones that are present. That's slightly worse if somebody actually sets all of the config (N+1), but much better if they don't (you end up with a single call).

@scottchiefbaker
Copy link
Contributor Author

This was addressed in the v1.4.1 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants