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

🐛 [format.pretty] breaks delta #174

Closed
boris-petrov opened this issue May 3, 2020 · 17 comments · Fixed by #558
Closed

🐛 [format.pretty] breaks delta #174

boris-petrov opened this issue May 3, 2020 · 17 comments · Fixed by #558

Comments

@boris-petrov
Copy link

Hi, thanks for making this! Still got a few bugs but once they are fixed, that would be an indispensable tool!

I have in my .gitconfig:

[format]
	pretty = %C(blue)%H%C(red) %an %C(green)%ad%C(yellow)%d%n%B
[core]
	pager = delta --commit-color=blue --file-color=yellow --file-style=box --hunk-color=purple --hunk-style=plain --theme=none

The [format] part seems to break delta. When I do a git log -p, the first line (commit hash, committer, date) is fine however the rest are colorless. The style is preserved as I've specified in format though. What can I do about it?

Delta 0.1.1.

@kbd
Copy link

kbd commented Sep 25, 2020

This is present in 0.4.3.

Here's a screenshot of my custom git log with a -p:

Screen Shot 2020-09-24 at 10 37 17 PM

The "install gomp" in the second commit in the screenshot is incorrectly rendered as if it's a diff, including line numbers.

@ghost
Copy link

ghost commented Oct 15, 2020

@boris-petrov

I think this one is already resolved?

ss 2

Not sure where it fixes, but maybe at version 0.1.1, this condition couldn't catch the Meta so that they don't continue.

delta/src/delta.rs

Lines 146 to 148 in 9732c88

if state == State::FileMeta && config.file_style != cli::SectionStyle::Plain {
// The file metadata section is 4 lines. Skip them under non-plain file-styles.
continue;

delta: v0.4.4

@boris-petrov
Copy link
Author

Yes, I believe this is fixed. Thanks!

@kbd
Copy link

kbd commented Oct 15, 2020

Confirmed the behavior is fixed in 0.4.4 vs 0.4.3. Thanks!

@kbd
Copy link

kbd commented Oct 15, 2020

I apologize, my earlier message was incorrect. I tested with git log -p, which always worked. My custom log format, with git l -p, still shows incorrect formatting:

Screen Shot 2020-10-15 at 10 36 19 AM

@kbd
Copy link

kbd commented Oct 15, 2020

The log format alias above is:

l = log --date=format:'%a %Y-%m-%d %k:%M' --color=always --pretty=format:'%C(green)%ad %C(blue)%an %C(auto)%h%d%n %s%n%w(0,4,4)%+b%C(reset)' --stat

@dandavison dandavison reopened this Oct 15, 2020
@ghost
Copy link

ghost commented Oct 15, 2020

@kbd
Ah, I know see that's a bug, thank you for detail.

@dandavison
I think this is caused because of this code.

delta/src/delta.rs

Lines 90 to 92 in 9cf186c

if line.starts_with("commit ") {
painter.paint_buffered_minus_and_plus_lines();
state = State::CommitMeta;

If I fix here forcibly (wrong approach. I'm doing like if line.contains(" ryuta69 ")), this can be fixed like below.
ss 4

Not sure how to detect if this is CommitMeta, but if I come up with good idea, will fix.

@Mellbourn
Copy link

What is broken for me is that the commit line is not formatted correctly for log -p and reflog -p (it does not get any "box").

The workaround I've found is to add the word commit in the log line. So configuring these git aliases in .gitconfig displays log -p and reflog -p correctly:

[alias]
	logp = log -p --format='commit %C(yellow)%h %C(magenta)%<(15,trunc)%an %C(cyan)%cd %C(auto)%d%Creset %s'
	reflogp = reflog -p --format='commit %C(auto)%h%d %gd: %gs (%s)'

@dandavison
Copy link
Owner

dandavison commented Apr 7, 2021

Hi @Mellbourn, by default git does include the word commit in git log -p output, so this should be working correctly for you (indeed, I use git log -p a lot). On the other hand I agree that delta isn't handling git reflog -p very well, as you say!

I wonder what's going on with your git log -p. Is it possible that you have another alias altering the default git log output?

@Mellbourn
Copy link

I should have mentioned: I have a custom format.pretty in my .gitconfig to make git log look good and be more concise. That format does not have the word commit in it:

[format]
	pretty = %C(yellow)%h %C(magenta)%<(15,trunc)%an %C(cyan)%cd %C(auto)%d%Creset %s

Sample output:
nicely formatted git log

@dandavison
Copy link
Owner

Ah, OK. Right, your example challenges delta to explain itself a bit! I mean, delta (being GIT_PAGER) has to handle every single line of output from git; not just diffs. And it promises to parse that output and decorate things. And yet, git itself encourages the structure of the output to be heavily customized. You can see that what delta does is extremely simple:

let mut handled_line = if line.starts_with("commit ") {

So, what shall we do? I'm open to delta having fairly crazy technical options -- it is a tool for programmers after all -- as long as it doesn't affect performance / UX in the default case. We could perhaps allow users to supply their own regular expression to match key sections of the output such as the commit line to be decorated? Or perhaps delta could inspect the format.pretty and derive a regular expression from that? But there's perhaps no way to make that work for [alias] entries? If anyone reading feels like this area sounds fun to have a go at, please do shout out.

@Mellbourn
Copy link

Sounds to me like we would get the most gain for the least effort by allowing users to supply their own regular expressions.

@dandavison
Copy link
Owner

Sounds to me like we would get the most gain for the least effort by allowing users to supply their own regular expressions.

Yes, I agree.

@dandavison
Copy link
Owner

dandavison commented Apr 8, 2021

That seems to have been a simple change. #558 if you would like to try it out. So for your example, something like git -c delta.commit-regex='^[0-9a-f]{7,} ' log should work.

image

@kbd
Copy link

kbd commented Apr 8, 2021

I've often thought it's a shame that the separator characters included in ASCII are never used. They'd be a natural fit here if git had used them to delimit structure in git log. Eg. if a record separator preceded every commit it would save ad-hoc "starts with 'commit '" parsing.

I suppose that'd be the best way to use the regex with a custom log format? Add a separator character to the beginning of your log format and look for that in the regex?

kbd added a commit to kbd/setup that referenced this issue Apr 8, 2021
@dandavison
Copy link
Owner

dandavison commented Apr 8, 2021

@kbd I think you could use a record separator character (thanks, I do sometimes use zero-width space for hacks but I wasn't familiar with the ASCII record separators). On the other hand something ad-hoc like this works for your l alias:

git -c delta.commit-regex='> [0-9a-f]{7,}$'

@dandavison
Copy link
Owner

commit-regex is released now (delta 0.8.0)

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 a pull request may close this issue.

4 participants