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 smartlog revset rendering #474

Closed
wants to merge 3 commits into from
Closed

Improve smartlog revset rendering #474

wants to merge 3 commits into from

Conversation

arxanas
Copy link
Owner

@arxanas arxanas commented Jul 27, 2022

No description provided.

@arxanas arxanas force-pushed the arxanas/revset branch 2 times, most recently from 46e9b6e to dbb440e Compare July 27, 2022 03:23
@arxanas arxanas force-pushed the arxanas/revset-2 branch 2 times, most recently from 8983a41 to 9fd5eb3 Compare July 27, 2022 05:16
@claytonrcarter
Copy link
Collaborator

Thank you for tinkering on this! Yes, I do find the output more intuitive. My couple of comments:

  • Probably just a draft/temp thing, but it looks like hidden commits are included in the revset-filtered output even if --hidden is not passed in.

  • Is it possible (or even desirable) to display the vertical ellipsis after commits in heads(revset) to indicate that that commit is not a head when the output is not filtered?

Unfortunately, I haven't come up with a way to support not showing the ancestor commits up to the main branch. Currently, it's a hard-coded assumption.

Would it be at all relevant to this problem (or feasible) to always show the HEAD commit, but to omit the commits between roots(revset) and HEAD and just display when w/ the vertical ellipsis? Maybe even always showing the current main branch commit?

Here's a mockup of what I'm wondering, given this smartlog:

⋮
◇ 0f06699 1d (master, remote origin/master) build: run `cargo upgrade`
┃
◯ 7e512e6 1d build(revset): build parser as part of `build.rs`
┃
◯ 87bc6fd 1d tests(eval): create `eval_and_sort` function
┃
◯ 96b891e 1d feat(revset): add `not` function
┃
◯ c9ec768 1d feat(revset): produce friendlier error messages
┣━┓
┃ ◯ 3793bfd 1d refactor(revset): rewrite grammar to support `:` as a mixfix operator
┃ ┃
┃ ◯ bc0c8b7 1d (arxanas/revset) feat(smartlog): support passing an arbitrary revset expression
┃
◯ 44282b9 7h refactor(revset): rewrite grammar to support `:` as a mixfix operator
┃
◯ dbb440e 7h feat(smartlog): support passing an arbitrary revset expression
┃
◯ 82c92b1 5h ci: raise test timeout
┃
◯ 57f00be 5h temp: smartlog improve branches() revset
┃
◯ d09305a 5h temp: set default revset
┃
● 9fd5eb3 5h (ᐅ arxanas/revset-2) temp: try to pass in references snapshot

Current

❯ cargo run -- smartlog 'children(c9ec768)'
⋮
◇ 0f06699 1d (master, remote origin/master) build: run `cargo upgrade`
┃
◯ 7e512e6 1d build(revset): build parser as part of `build.rs`
┃
◯ 87bc6fd 1d tests(eval): create `eval_and_sort` function
┃
◯ 96b891e 1d feat(revset): add `not` function
┃
◯ c9ec768 1d feat(revset): produce friendlier error messages
┣━┓
┃ ◯ 3793bfd 1d refactor(revset): rewrite grammar to support `:` as a mixfix operator
┃
◯ 44282b9 7h refactor(revset): rewrite grammar to support `:` as a mixfix operator
┃
◯ dbb440e 7h feat(smartlog): support passing an arbitrary revset expression
┃
◯ 82c92b1 5h ci: raise test timeout
┃
◯ 57f00be 5h temp: smartlog improve branches() revset
┃
◯ d09305a 5h temp: set default revset
┃
● 9fd5eb3 5h (ᐅ arxanas/revset-2) temp: try to pass in references snapshot

Maybe? 🤷

❯ cargo run -- smartlog 'children(c9ec768)'
⋮
◇ 0f06699 1d (master, remote origin/master) build: run `cargo upgrade`
⋮
┣━┓
┃ ◯ 3793bfd 1d refactor(revset): rewrite grammar to support `:` as a mixfix operator
┃ ⋮
┃
◯ 44282b9 7h refactor(revset): rewrite grammar to support `:` as a mixfix operator
⋮
● 9fd5eb3 5h (ᐅ arxanas/revset-2) temp: try to pass in references snapshot

Base automatically changed from arxanas/revset to master July 31, 2022 20:52
@arxanas
Copy link
Owner Author

arxanas commented Aug 8, 2022

Haven't had time to work on this, so closing for now.

@arxanas arxanas closed this Aug 8, 2022
@arxanas arxanas deleted the arxanas/revset-2 branch August 8, 2022 21:42
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