Sertion / vscode-gitblame

Visual Studio Code Extension - See Git Blame info in status bar.
https://marketplace.visualstudio.com/items/waderyan.gitblame
MIT License
72 stars 31 forks source link

Allow providing custom arguments to the invocation git blame #165

Closed mpawlowski-eyeo closed 5 months ago

mpawlowski-eyeo commented 5 months ago

In my project I have a graft file to stitch together a history that's broken by rebases.

To allow git blame to track the history, I need to call it as git blame -S grafts.txt -- {path}

AFAIU there is currently no option to provide custom arguments to the invocation of git blame so I can't tell it to use my grafts.

Alternatively, specifying a custom alias for blame would work - I can call git my-blame {path} and have an alias defined in .git/config that my-blame = blame -S grafts.txt.

Either would work for my use case.

Sertion commented 5 months ago

Hey mpawlowski-eyeo. Thanks for the feature request!

The revs-file sounds like it is unique per repository. The git blame process is always executed with a cwd set to the directory of the file it is currently blaming. That would mean that the revs-file path would need to be either relative to the .git-folder it was built from or an absolute path. This could possibly interfere with the blame information for sub repositories. Any idea how they interact?

mpawlowski-eyeo commented 5 months ago

That's a good point!

One way to work around that is to declare the setting like:

"gitblame": {
  "extraCmdArguments": [
    "-S",
    "${workspaceFolder}/grafts.txt"
  ]
}

This makes the path absolute AFAIR?

This likely wouldn't help for the second, alias-based approach, because git my-blame would be an unknown command in a sub repository - unless that alias was a global config or the setting was repeated in the sub repo's .git/config.

The grafts.txt file, provided through an absolute path via ${workspaceFolder}/, would not be meaningful for sub repositories because the commit hashes don't exist in their histories, but this wouldn't break anything (unless there was an astronomically unlikely random hash collision). As long as the hashes from the file aren't encountered in the history traversal that git blame does, they're dormant.

FWIW I found the content/format of the revs-file passed via -S to be poorly documented, and after some googling I found that it looks like this:

$ cat git-grafts.txt 
1993fb58c5732068109ce4178ee09bced4973a8a 83a714deb7aeff970e55ef11174b03aa02970d29
3cc64cebef5c028d87495de797630d2e3d56d522 a71c84f53e4e2254f6a34ed50e37ecf5d4362eb9
85b8322d2c1c97e7adfbe2162e7ade16fcd1e019 f2299e697fb99f2a693b405a2043d9622ebfe68b
075b1b8375709eae8a56081a9d4230ab56484ba4 65212eee6e9e6043ad78fc59619cbba31a692f67
e5425abe809325552de52772f2c5f379060f41d8 b78c67a90f7e0ed9ee2529da85520ba9d24185c1
...

When git blame traverses history and reaches a commit 1993fb58c5732068109ce4178ee09bced4973a8a it will skip over it and go to 83a714deb7aeff970e55ef11174b03aa02970d29, then resume traversing history from there. If it then reaches 3cc64cebef5c028d87495de797630d2e3d56d522 it will skip over to a71c84f53e4e2254f6a34ed50e37ecf5d4362eb9 and so on. This is useful if your project has been rebased or resquashed and the "real" history is scattered across many branches. The "skipped" commits are squash commits that would otherwise become a dead end.

Sertion commented 5 months ago

Thanks for the explanation of the -S command! It was a bit unclear.

The ${workspaceFolder} idea looks good on the surface, but then you run into a situation like the following:

d  .git
f  .gitmodules
f  grafts.txt
d  a-git-module
    d  .git
    f  module-grafts.txt

(d) directory  (f) file

Perhaps it is not worth figuring out a solution that will work for this case as I get a feeling that this is an extreme edge case.

Workspaces also don't work perfectly as the extension no longer requires a workspace to be able to blame files.

Perhaps the only way to do it is to acknowledge these issues, document them and accept them.

mpawlowski-eyeo commented 5 months ago

I guess alias support could be a workaround for that case, albeit not super pretty:

One way:

# global scope, "grafted-blame" alias forwards to "blame"
git config --global alias.grafted-blame = blame

# top-level repo, local config overrides global config, the alias now uses grafts.txt
cd repo/
git config --local alias.grafted-blame = blame -S grafts.txt

# submodule, the alias uses module's grafts
cd a-git-module/
git config --local alias.grafted-blame = blame -S module-grafts.txt

# git grafted-blame {path} now behaves sensibly everywhere

You could also use conditional includes in the config files or even run a custom bash script that selects the correct graft file as part of the alias. Git aliases are pretty powerful, it turns out. But these are all advanced options, the -S {workspaceFolder}/grafts.txt approach is easier to set up and probably handles many common use cases (considering grafts aren't that common in the first place).

I suppose we could have both features: custom command line args for blame and the option to use a custom alias instead of blame

Sertion commented 5 months ago

I'm not interested in allowing custom commands as that opens up a hole can of worms when it comes to ACE-exploits.

How useful would a setting that looks for a file relative to the .git directory? The value of the setting would be something like ./git-grafts.txt that would find a file in the following structure:

d  .git
f  git-grafts.txt

(d) directory  (f) file

It has a few upsides:

mpawlowski-eyeo commented 5 months ago

I think that would work for my use case, though I need to be able to specify a multi-component path (tools/something/grafts.txt). The path can be assumed to be relative to repo root.

This path will not exist for submodules (which I have in my use case) and passing a non-existent path trips git blame:

$ git blame -S does-not-exist.txt README.md
fatal: reading graft file 'does-not-exist.txt' failed: No such file or directory
Sertion commented 5 months ago

It is luckily quite easy to check if a file exists before adding the -S-flag. I will see when I have time to implement this. Hopefully it will be sometime this week.

Thanks for the dialog. It has been very helpful!

Sertion commented 5 months ago

A new version has been uploaded to the marketplaces. It can sometimes take a day or two before it is pushed to everyone. Thanks for the feature request. I hope that it helps with your use case!

mpawlowski-eyeo commented 5 months ago

Thanks a lot! I'll check it out on Monday and let you know :)

sob., 20 sty 2024, 17:07 użytkownik Albin Jacobsson < @.***> napisał:

A new version has been uploaded to the marketplaces. It can sometimes take a day or two before it is pushed to everyone. Thanks for the feature request. I hope that it helps with your use case!

— Reply to this email directly, view it on GitHub https://github.com/Sertion/vscode-gitblame/issues/165#issuecomment-1902163578, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARRMERH4XCXCGSFZIP6WAATYPPTU3AVCNFSM6AAAAABBVGCE6KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBSGE3DGNJXHA . You are receiving this because you authored the thread.Message ID: @.***>

mpawlowski-eyeo commented 5 months ago

It works fine, great job :)