search

gitkraken / vscode-gitlens

Supercharge Git inside VS Code and unlock untapped knowledge within each repository — Visualize code authorship at a glance via Git blame annotations and CodeLens, seamlessly navigate and explore Git repositories, gain valuable insights via rich visualizations and powerful comparison commands, and so much more
http://gitkraken.com/gitlens
Other
9k stars 1.34k forks source link

Option to list commits in topological order #1257

Closed laanwj closed 3 years ago

laanwj commented 3 years ago

It looks like gitlens always sorts the commits list by date, as github does. However to me this is inconvenient when reviewing a PR consisting of multiple commits because different commits in a 'train' of commits are interleaved.

So it would be nice to have a way to change this. This would be the equivalent to

git log --topo-order
eamodio commented 3 years ago

Sounds like a good idea. Would you mind opening a PR to add that or get it started? I'd be happy to help point in the right direction, etc.

thewindsofwinter commented 3 years ago

Hi, I was wondering if there was still someone working on this: it doesn't seem too hard to implement. If not, I could work on it, although I'm not the most familiar with open-source and gitlens so I might need some guidance in the right direction. Thanks! ~andy

eamodio commented 3 years ago

@thewindsofwinter 👋 — as far as I know, no one is working on this.

So the first question to answer is -- is this a global thing? Like should GitLens have a setting that lets you change to --topo-order for any git log operation? I'm thinking that is the case, but we should verify that first. @laanwj thoughts?

Assuming we are going to have a global setting, here are a couple pointers.

To add a new setting you'll need to add it to the package.json, e.g.: https://github.com/eamodio/vscode-gitlens/blob/369cf3ea3e0bb8b7662e3225421c70f5d8433634/package.json#L2348-L2353

And here: https://github.com/eamodio/vscode-gitlens/blob/ae03319ab1e8d8a0edce629ec08513de3e26cdb4/src/config.ts#L279-L281

Then in git.ts, you'll need to search for all the functions that call git log, by searching for 'log', e.g. https://github.com/eamodio/vscode-gitlens/blob/20b1920bacf943d70add6ad8a788ded73fc10eeb/src/git/git.ts#L740

Then for each of those functions, plumb through a new parameter to control the new option. And finally, find all the references to those functions (in gitService.ts, and pass through the new option using the config setting, e.g. https://github.com/eamodio/vscode-gitlens/blob/ae03319ab1e8d8a0edce629ec08513de3e26cdb4/src/git/gitService.ts#L1789-L1813

That should get you going in the right direction.

laanwj commented 3 years ago

@thewindsofwinter I'm not currently working on this.

thewindsofwinter commented 3 years ago

@eamodio alright, I've forked and I'll start following the steps you specified, assuming @laanwj would like a global option.

thewindsofwinter commented 3 years ago

@eamodio I think I've done everything that you've specified, however I'm a little confused about some of the stuff on the pull request template, specifically: "My changes include any required corresponding changes to the documentation"... is there any documentation I have to update about this? Also, I linted, and I think nothing's showing up in the problems besides a bunch of unknown settings in VSCode -- does that mean that's good to go? Apologies for all the questions, still learning how this works :D

eamodio commented 3 years ago

There is now a new gitlens.advanced.commitOrdering setting, which can be set to date, author-date, or topo.

Can you please verify this fix in tomorrow's insiders edition?

You can install the insiders edition from here. Be sure to disable/uninstall the stable version of GitLens first.

thewindsofwinter commented 3 years ago

There is now a new gitlens.advanced.commitOrdering setting, which can be set to date, author-date, or topo.

Can you please verify this fix in tomorrow's insiders edition?

You can install the insiders edition from here. Be sure to disable/uninstall the stable version of GitLens first.

@eamodio I would be glad to! However, I'm not sure that I've done anything fancy enough in Git really to test this properly: what things should I try to test it well? (I'm thinking of creating a couple branches, making a bunch of basic commits, and merging them a bunch and viewing it in each of the modes?)

eamodio commented 3 years ago

Great, thank you! Yeah, I think something like that should work. See here: https://git-scm.com/docs/git-log#Documentation/git-log.txt---topo-order

laanwj commented 3 years ago

Thank you for implementing this!

@eamodio I would be glad to! However, I'm not sure that I've done anything fancy enough in Git really to test this properly: what things should I try to test it well? (I'm thinking of creating a couple branches, making a bunch of basic commits, and merging them a bunch and viewing it in each of the modes?)

To reproduce this in a test repository, maybe: create commits on two branches in alternating fashion, then merge the two branches into the main branch. The commits should appear interleaved in date-based sorting but distinct in topological sorting.

I just installed the gitlens insiders edition but couldn't find the setting yet. I guess I am too early and this is not tomorrow's edition yet :smile:

screenshot_vscode_noopt

thewindsofwinter commented 3 years ago

There is now a new gitlens.advanced.commitOrdering setting, which can be set to date, author-date, or topo.

Can you please verify this fix in tomorrow's insiders edition?

You can install the insiders edition from here. Be sure to disable/uninstall the stable version of GitLens first.

I just verified: it is there, apologies for the lateness of my reply. The setting looks great and after testing it on a toy repository everything seems to work! Thank you @eamodio :D

eamodio commented 3 years ago

Thank YOU for the contribution and verification!

laanwj commented 3 years ago

Today I did get the update, and was able to see and change the gitlens.advanced.commitOrdering setting.

This works perfectly now, thanks!

github-actions[bot] commented 3 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.