Jump to commit via sha #1818

AmmarAbouZor · 2023-08-20T18:44:36Z

This Pull Request fixes/closes #1799

This PR stills work in progress

It changes the following:

Adds new function the commits-log to jump to commit via its sha
Default Keybinding is currently Ctrl-j
The input pupop has live validation
After closing the input box it'll jump to the commit with the matching SHA

The following point need clarification:

The live validation retrieve the repo with each keystroke to check if the sha is valid via the method git2::revparse_single(). I've tested it with big repos and didn't see any performance hit with it. But we still can make simple live validation (only hash chars, no spaces, length at most 40) If the current implementation's performance isn't good enough.
The default keybinding
Validation error text
Title and placeholder texts

I'll add a demo for the current progress

I followed the checklist:

I added unittests
I ran make check without errors
I tested the overall application
I added an appropriate item to the changelog

Struct is created with a default keybindings. Popup in integrated in the initialisation macros

- implement default components methods on the popup - Create internal event for jump to commit

- Popup is reactive and has the standard key bindings - Validation is still not implemented - Past from clipboard is still not implemented

- Basic Validation is implemented - Jump to comment with full sha is implemented without error handling

- Handle move to commit errors

- Validation error message improvements - Method renamed

AmmarAbouZor · 2023-08-20T18:51:47Z

Here is a small demo for the current progress

jump_sha.mp4

Demo isn't used in the PR

joyously · 2023-08-20T18:59:52Z

Does it jump to it even if that SHA is not in the visible list?

AmmarAbouZor · 2023-08-20T19:06:36Z

@joyously Yes it jumps to any commit in the repository since it uses the well-tested method select_commit() in revlog

- Add redraw flag after it jumps to commit - Remove unused code

extrawurst · 2023-08-21T13:16:03Z

Does it jump to it even if that SHA is not in the visible list?

yes it will only be able to go to commits in the current branch. otherwise it will show an error right now. #81 will introduce looking at all branches and fix this

extrawurst · 2023-08-21T13:18:27Z

@AmmarAbouZor this looks great.

To not add yet another command to the log view I would like this to be an option in the new search popup LogSearchPopupComponent. Logically it also makes sense to be in there. once this in place I will merge this.

AmmarAbouZor · 2023-08-22T09:50:28Z

@extrawurst I think it would be better in the search popup too.
I'd add a new option there Commit SHA Or Commit Hash and move the implementation there.
Could you please have a look at #1816 before. This would save me one merge conflict 😉

extrawurst · 2023-08-22T10:47:38Z

#1816 is merged

- Jump to commit is embedded into search commit popup - Popup strings are also deleted

- Rename related fields and methods from search commit hash to jump commit

AmmarAbouZor · 2023-08-23T08:16:22Z

@extrawurst Jump to SHA is embedded in LogSearchPopupComponent.

I added a new mode there with the name 'commit hash'
No other option can be active along with the commit hash mode
The validation still happens but it's not visible to the users. No execution can happened if the commit hash mode is active and no valid SHA exists

jump_sha.mp4

extrawurst · 2023-08-23T19:56:55Z

Ok I'll get back to the previous implementation

but do not add a new popup. Reuse this one please. Make it use a mode neun or something to store what mode it is in at the moment

- Validation isn't visualize yet

- Event handling is done in each mode independently to reduce code complexity

AmmarAbouZor · 2023-08-24T13:23:51Z

@extrawurst I've built the different modes behavior in LogSearchPopupComponent.

With the default key f the popup will open in the normal search mode.
With the default key Ctrl-j the popup will open in the jump to commit mode.
Jump to commit mode is just the input box with a validation shown on the border only

I think it would be better to define how the validation visualization should look before it's implemented.

Currently only the border will turn red if it's invalid
Should the text in the input field be red in the case of invalid too?
The current colors pallet doesn't have a color for valid input. It has only the danger-fg which is used for the invalid case.

jump_sha.mp4

joyously · 2023-08-24T13:47:50Z

For accessibility, it's important that information is not conveyed only with color. (7% of men are red/green color blind)
In the demo, it looked like less than 3 characters was considered invalid, but it matched at 4. Perhaps a better UI would show the matching hashes, which the user could pick from, although that might be a bit much for a terminal... (but the whole point is that the hash is not visible)

AmmarAbouZor · 2023-08-24T15:20:21Z

For accessibility, it's important that information is not conveyed only with color. (7% of men are red/green color blind)
In the demo,

We can show a validation message in the right bottom corner like in the commit popup. (first iteration of this PR used that)

Considering that the users won't type the hashes themselves in the most cases but they will paste from the clipboard, I don't think that implementing an auto-completion here would add more value.
I can though complete the SHA in the textbox once the users have typed enough characters to determine one single commit, but this can confuse the users in case they are typing a commit that doesn't exist in the repository

asyncgit/src/sync/commits_info.rs

src/app.rs

src/keys/key_list.rs

src/queue.rs

src/strings.rs

src/tabs/revlog.rs

src/components/log_search_popup.rs

extrawurst · 2023-08-26T11:37:46Z

back to the previous implementation

that was not the idea. the command for jump-to-commit-sha should be only available in the search-popup to prevent littering the revlog command list more. it contextually also makes sense to be a sub behaviour of find/search.

also please add the new command in the cmd-bar so its clear that this feature is available

We can show a validation message in the right bottom corner like in the commit popup

lets do that

complete the SHA in the textbox once the users have typed enough characters

no magic please. for the unlikely case that people want to search for incomplete/fractional SHAs we can add that to the regular substring search

- Jump to commit is now a sub-command in log_search_popup - Remove redundant JumpToCommit Internal Event and use SelectCommitInRevlog instead

- Rename jump_to_commit_hash to search_commit_sha - Remove Jump to commit title and use search instead - Change find_commit_sha group

- Mode switch is done internally - Switch to jump commit is always enabled - Esc in jump commit mode return to search mode

src/strings.rs

AmmarAbouZor · 2023-08-27T07:30:30Z

Commit sha command is shown in LogSearchPopup with the default keybinding Ctrl-j
Validation message is shown on the right bottom corner when the sha is invalid only
Esc in Commit SHA will go back the search popup

jump_sha.mp4

extrawurst · 2023-08-27T07:46:36Z

Thanks for sticking with this. Good work! ❤️

AmmarAbouZor added 9 commits August 19, 2023 20:49

Start integrating jump to commit popup in the app

0e7cbd0

Struct is created with a default keybindings. Popup in integrated in the initialisation macros

Building & Integrating Jump to Commit in the app

ac6421d

- implement default components methods on the popup - Create internal event for jump to commit

Implementing jump to commit jump commit popup

5d08c09

- Popup is reactive and has the standard key bindings - Validation is still not implemented - Past from clipboard is still not implemented

Implementing Jump to commit

a0bbf70

- Basic Validation is implemented - Jump to comment with full sha is implemented without error handling

Jump To Commit: Add error handling

5989c24

- Handle move to commit errors

Jump to commit: short revs can be used

a6bc97c

Jump to commit: Text improvements & refactoring

a6253a9

- Validation error message improvements - Method renamed

Jump to Commit: Change default keybinding

648efe0

Jump to commit: Added demo

03170a1

AmmarAbouZor added 2 commits August 20, 2023 20:54

Jump to commit: Remove unused demo

a209381

Demo isn't used in the PR

Jump to commit: Clippy Fixes

9a12404

Jump to commit: Redraw after jump

70cfbab

- Add redraw flag after it jumps to commit - Remove unused code

Merge branch 'master' into jump-to-commit-via-sha

0f2c67b

extrawurst mentioned this pull request Aug 22, 2023

View log at git revision #1600

Closed

AmmarAbouZor added 8 commits August 22, 2023 19:16

Merge branch 'master' into jump-to-commit-via-sha

a5bd646

Embed jump to commit hash in log search popup

1409f93

Remove jump to commit popup

18b82a8

- Jump to commit is embedded into search commit popup - Popup strings are also deleted

Small refactoring: Remove unnecessary brackets

1e0482f

Jump to commit: Refactoring

d6d7d55

- Rename related fields and methods from search commit hash to jump commit

Jump to Commit Refactoring: change arguments order

082af1d

Jump to commit: Add unit test

3d9ab22

Jump to commit: Add item to CHANGELOG

6eaa68d

AmmarAbouZor marked this pull request as draft August 23, 2023 19:10

AmmarAbouZor added 6 commits August 24, 2023 11:16

Embed Jump Commit in search popup as separate mode

984ba9c

- Validation isn't visualize yet

Remove Jump to commit separate component again

f2a5137

Jump to commit: Separate event handling

57c7ad8

- Event handling is done in each mode independently to reduce code complexity

Jump to Commit: Apply validation visualization

1b807d3

Fix log search popup text box highlighting

8d46893

Jump to Commit: Clippy Fixes

62a5e49

extrawurst requested changes Aug 26, 2023

View reviewed changes

AmmarAbouZor added 9 commits August 26, 2023 16:01

Merge branch 'master' into jump-to-commit-via-sha

1042d3c

Add scope_time to CommitId::from_revision

fd68863

Jump to Commit: Changes after review

9abfc2f

- Jump to commit is now a sub-command in log_search_popup - Remove redundant JumpToCommit Internal Event and use SelectCommitInRevlog instead

Jump to Commit: Changes after review

e9263d8

- Rename jump_to_commit_hash to search_commit_sha - Remove Jump to commit title and use search instead - Change find_commit_sha group

Find Commit Sha: Use RepoPathRef

541af84

Rewire LogSearchPopup mode switch logic

1a25f99

- Mode switch is done internally - Switch to jump commit is always enabled - Esc in jump commit mode return to search mode

Commit Sha Refactoring: Unify names

7dccae8

Show validation message when sha is invalid

36dce88

Commit SHA: Clippy fixes

9c034b3

extrawurst reviewed Aug 27, 2023

View reviewed changes

src/strings.rs Outdated Show resolved Hide resolved

Commit SHA: Change command text

df70cee

AmmarAbouZor marked this pull request as ready for review August 27, 2023 07:34

extrawurst merged commit c68fa3e into extrawurst:master Aug 27, 2023
19 checks passed

AmmarAbouZor deleted the jump-to-commit-via-sha branch August 27, 2023 08:36

IndianBoy42 pushed a commit to IndianBoy42/gitui that referenced this pull request Jun 4, 2024

Jump to commit via sha (extrawurst#1818)

94ddd30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Jump to commit via sha #1818

Jump to commit via sha #1818

AmmarAbouZor commented Aug 20, 2023 •

edited

Loading

AmmarAbouZor commented Aug 20, 2023

joyously commented Aug 20, 2023

AmmarAbouZor commented Aug 20, 2023 •

edited

Loading

extrawurst commented Aug 21, 2023

extrawurst commented Aug 21, 2023

AmmarAbouZor commented Aug 22, 2023

extrawurst commented Aug 22, 2023

AmmarAbouZor commented Aug 23, 2023

extrawurst commented Aug 23, 2023

AmmarAbouZor commented Aug 24, 2023

joyously commented Aug 24, 2023

AmmarAbouZor commented Aug 24, 2023

extrawurst commented Aug 26, 2023 •

edited

Loading

AmmarAbouZor commented Aug 27, 2023

extrawurst commented Aug 27, 2023

Jump to commit via sha #1818

Jump to commit via sha #1818

Conversation

AmmarAbouZor commented Aug 20, 2023 • edited Loading

AmmarAbouZor commented Aug 20, 2023

joyously commented Aug 20, 2023

AmmarAbouZor commented Aug 20, 2023 • edited Loading

extrawurst commented Aug 21, 2023

extrawurst commented Aug 21, 2023

AmmarAbouZor commented Aug 22, 2023

extrawurst commented Aug 22, 2023

AmmarAbouZor commented Aug 23, 2023

extrawurst commented Aug 23, 2023

AmmarAbouZor commented Aug 24, 2023

joyously commented Aug 24, 2023

AmmarAbouZor commented Aug 24, 2023

extrawurst commented Aug 26, 2023 • edited Loading

AmmarAbouZor commented Aug 27, 2023

extrawurst commented Aug 27, 2023

AmmarAbouZor commented Aug 20, 2023 •

edited

Loading

AmmarAbouZor commented Aug 20, 2023 •

edited

Loading

extrawurst commented Aug 26, 2023 •

edited

Loading