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

Option to list commits in topological order #1257

Closed
laanwj opened this issue Dec 10, 2020 · 14 comments · Fixed by #1344
Closed

Option to list commits in topological order #1257

laanwj opened this issue Dec 10, 2020 · 14 comments · Fixed by #1344
Assignees
Labels
feature New feature or request good first issue Good for newcomers help wanted Want community input and/or pull request verified ✔ Verified
Milestone

Comments

@laanwj
Copy link

laanwj commented Dec 10, 2020

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
@laanwj laanwj added the feature New feature or request label Dec 10, 2020
@eamodio eamodio added good first issue Good for newcomers help wanted Want community input and/or pull request labels Dec 10, 2020
@eamodio eamodio added this to the Backlog milestone Dec 10, 2020
@eamodio
Copy link
Member

eamodio commented Dec 10, 2020

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
Copy link
Contributor

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
Copy link
Member

eamodio commented Jan 19, 2021

@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
Copy link
Author

laanwj commented Jan 19, 2021

@thewindsofwinter I'm not currently working on this.

@thewindsofwinter
Copy link
Contributor

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

@thewindsofwinter
Copy link
Contributor

@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

thewindsofwinter added a commit to thewindsofwinter/vscode-gitlens that referenced this issue Jan 26, 2021
eamodio pushed a commit to thewindsofwinter/vscode-gitlens that referenced this issue Mar 29, 2021
…oses gitkraken#1257: option to use commitOrdering

- commitOrdering setting will now allow to order the commits by date, author or topo.
eamodio added a commit to thewindsofwinter/vscode-gitlens that referenced this issue Mar 29, 2021
eamodio pushed a commit to thewindsofwinter/vscode-gitlens that referenced this issue Mar 29, 2021
eamodio pushed a commit to thewindsofwinter/vscode-gitlens that referenced this issue Mar 29, 2021
…oses gitkraken#1257: option to use commitOrdering

- commitOrdering setting will now allow to order the commits by date, author or topo.
eamodio added a commit to thewindsofwinter/vscode-gitlens that referenced this issue Mar 29, 2021
eamodio added a commit that referenced this issue Mar 29, 2021

Co-authored-by: Shashank-Shastri <shashank.m.shastri@gmail.com>
Co-authored-by: Eric Amodio <eamodio@gmail.com>
eamodio added a commit that referenced this issue Mar 29, 2021
Co-authored-by: Shashank-Shastri <shashank.m.shastri@gmail.com>
Co-authored-by: Eric Amodio <eamodio@gmail.com>
@eamodio eamodio added needs-verification Request for community verification pending-release Resolved but not yet released to the stable edition labels Mar 29, 2021
@eamodio eamodio self-assigned this Mar 29, 2021
@eamodio eamodio modified the milestones: Backlog, Soon™ Mar 29, 2021
@eamodio
Copy link
Member

eamodio commented Mar 29, 2021

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
Copy link
Contributor

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
Copy link
Member

eamodio commented Mar 29, 2021

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
Copy link
Author

laanwj commented Mar 29, 2021

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 😄

screenshot_vscode_noopt

@thewindsofwinter
Copy link
Contributor

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 eamodio added verified ✔ Verified and removed needs-verification Request for community verification labels Apr 2, 2021
@eamodio
Copy link
Member

eamodio commented Apr 2, 2021

Thank YOU for the contribution and verification!

@laanwj
Copy link
Author

laanwj commented Apr 5, 2021

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

This works perfectly now, thanks!

@eamodio eamodio removed the pending-release Resolved but not yet released to the stable edition label Apr 9, 2021
@eamodio eamodio modified the milestones: Soon™, Shipped Apr 9, 2021
@github-actions
Copy link

github-actions bot commented May 9, 2021

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature New feature or request good first issue Good for newcomers help wanted Want community input and/or pull request verified ✔ Verified
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants