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: Append (You) to git blame author #21119

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

HarshNarayanJha
Copy link
Contributor

@HarshNarayanJha HarshNarayanJha commented Nov 23, 2024

This commit suffixes (You) to the git blame author if git config user.name is same as author name get_git_user_name fetches user.name, behaviour is controlled by the setting git.inline_blame.author_display

for_path method is modified to replace author name with the appropriate string.

Closes #10557

Release Notes:

  • Improved git blame author display to show (You) on author's machine controlled via the setting git.inline_blame.author_display

My Commit shows as ... (You)

image

...Rest doesn't

image

this commit suffixes (You) to the git blame author if `git config user.name` is same as author name
`get_git_user_name` fetches user.name
`for_path` method is modified to append (You) at the end of entry.author if author == username
@maxdeviant maxdeviant changed the title git: append (You) to git blame author git: Append (You) to git blame author Nov 23, 2024
@maxdeviant
Copy link
Member

Could you please include a screenshot of this change?

@maxdeviant maxdeviant self-assigned this Nov 23, 2024
@SomeoneToIgnore SomeoneToIgnore added the cla-signed The user has signed the Contributor License Agreement label Nov 23, 2024
@HarshNarayanJha
Copy link
Contributor Author

Ah, right! I clicked but forgot to post 😝

@HarshNarayanJha
Copy link
Contributor Author

Okay so what about adding a config option to show

  1. Author
  2. Author with You
  3. just You

Originally posted by @HarshNarayanJha in #10557 (comment)

Copy link
Member

@maxdeviant maxdeviant left a comment

Choose a reason for hiding this comment

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

I’m personally not a fan of this. I think having the “You” is unnecessary and adds extra noise.

If we do want to add this, it will need to be behind a setting that is disabled by default.

@HarshNarayanJha
Copy link
Contributor Author

HarshNarayanJha commented Nov 25, 2024

If we do want to add this, it will need to be behind a setting that is disabled by default.

That is what I suggested in the issue thread. I will add a config option under git.inline_blame named author_format with values

  • author
  • you
  • author_you

with

{
	"git": {
		"inline_blame": {
			"author_format": "author"
		}
	}
}

Being the default.
Suggestions for better config names are open. Also, is it better to put it under inline_blame, since it won't show up only in inline blame, but everywhere in the blame.

I will suggest changing the format a little bit to better clarify things up.

{
	"git": {
		"blame": {
			"inline": {
				"enabled": true,
				"delay_ms": 500,
				"show_commit_summary": true
			}
			"author_format": "author_you"
		}
	}
}

…display`. It defines how to display the author in git blame displays.

add new enum `GitAuthorDisplaySetting` with values `Author` (default), `You`, `AuthorYou`

Author -> Simply the default behaviour, e.g. `Linus Torvalds`
You -> Shows `You` inplace of author name on your machine. e.g. `You`
AuthorYou -> Both e.g. `Linus Torvalds (You)`

currently there is one issue. updating the setting doesn't reflect rendered blames. need to toggle inline blames to take effect.
@HarshNarayanJha
Copy link
Contributor Author

So it seems like I can't read setting from the git crate.

I had to go up to crates/git/src/repository.rs and modify the blame fn on GitRepository to accept an author_display_replace: Option<&str> and pass to the crate::blame::Blame::for_path( call, and for_path now uses this replacement option &str to handle replacements.

The repository receives that str from crates/project/src/buffer_store.rs's repo.blame call.

Now, here is the problem with the current implementation. The settings work as intended, default being the only author display. But changing the setting in the file doesn't update the blames immediately, but only after I toggle inline blame.

I think this is because of how I read the setting.

https://github.com/HarshNarayanJha/zed/blob/4d4aec2420fe03a3e8daf575f160adc04c872e78/crates/project/src/buffer_store.rs#L1129-L1149

Please check @maxdeviant

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inline git blame author (you logic)
3 participants