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

Improve blame timestamp handling #925

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

th1000s
Copy link
Collaborator

@th1000s th1000s commented Jan 19, 2022

If the timestamp is understood by the (now expanded) regex, but does not conform to the timestamp format then just print it as-is.

@dandavison
Copy link
Owner

dandavison commented Jan 20, 2022

Should this work? (It doesn't seem to)

git -c delta.blame-timestamp-format='%Y-%m-%d' blame --date='format:%Y-%m-%d' src/handlers/hunk_header.rs

@dandavison
Copy link
Owner

dandavison commented Jan 20, 2022

Thanks for doing this, my first pass at blame handling was definitely a work-in-progress!

Do you think it could be appropriate to inspect the git config blame.date key, and also look for --date among the calling process arguments, to automatically set the format, or is that going too far? (We have the machinery sitting there for both of course)

@th1000s
Copy link
Collaborator Author

th1000s commented Jan 20, 2022

'%Y-%m-%d'

That is because DateTime also requires a the time component of the input.

Parsing the exact blame key should work (is this in the git config data which delta has read already?). For even more customized date formats I've been thinking about parsing arbitrary strings like GNUs date --date="last week" does. However I don't think these corner cases are worth it. Instead what about trying to parse Author + Date in the regex, and if that fails using both together ( (author) (date) | (.*) ), dropping the {timestamp} field and using the fallback string in the {author} field (and ensuring each blame-format placeholder is unique, just as {n} must be in blame-separator-format.

If a user customized their blame timestamp this heavily, they might want to see it in delta as well.

If the timestamp is understood by the (now expanded) regex, but
does not conform to the timestamp format then just print it as-is.
@th1000s th1000s changed the title Make blame-timestamp-format actually useful Improve blame timestamp handling Sep 8, 2024
@th1000s
Copy link
Collaborator Author

th1000s commented Sep 8, 2024

Now accepts most timestamp formats, but --date=human or -s is still not accepted.

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