gitextensions
gitextensions copied to clipboard
gitextensions
git-grep UI
Fixes #4912 Depends on #11590 , only 92d937e21c88295b37be2f760ce91560c2e9ddb3 (and forward if added later) to be reviewed
Proposed changes
An UI for git-grep, to search commits without checking out those commits. (When working with this I have often used it to view worktree too.) https://git-scm.com/docs/git-grep
The UI in this PR adds a popup as well as a searchbox to the Diff tab.
The searchbox dropdown has the search history for the GE session, not persistent. The popup and searchbox are kept in sync.
Search in the popup is done when button is pressed (or Enter),
the diff tab box is searching when typing (similar in the filter).
If there is no -e in the search string, this is added to the search string.
Most git-grep options can be added, including --and etc.
git-grep options to handle case insensitive is available, but most options can be added in the searchbox.
Persistent options can be added too for instance --show-function.
The output of git-grep is always for a commit, including index and worktree. Note that the selected file in the grep group is not always followed when a new commit is selected, as it is not the primary group. This could be specially handled (but "grep" knowledge need to be added to RevDiff).
Performance In the Git repo, the git-grep requires about 200 ms for me, the information is added async after normal diff so no visible. For the Linux repo, git-grep can require several seconds. As the current search is aborted when a new is started, works fine for me. The processing in the fileviewer is faster than for normal diffs.
Limitations are basically the same as #11590 ,missing
- Documentation for the changes, how colors etc can be changed by Git config
- tests for the new functionality. May be postponed to a later PR
Screenshots
After
The popup window and the optional FileStatusList combobox
Example usage.
Test methodology
Manual
Merge strategy
squash.
:black_nib: I contribute this code under The Developer Certificate of Origin.
Just a point. Not saying git grep shouldn't be used. Just saying ripgrep is very speedy. So if searching working directory vs git history.
Just a point. Not saying git grep shouldn't be used. Just saying ripgrep is very speedy. So if searching working directory vs git history.
To clarify if someone is wondering: This feature is primarily to search Git commit history without a checkout.
This feature is not really competing with searching in the worktree. Searching in the editor and specific tools will normally be a better choice. I doubt anyone want to implement a GUI with all search options even (command line options can be provided though). I have ended up using the feature in the worktree too (for instance a quick way to search and exclude submodules and ignored files) but this is a sideeffect.
If noone has any other preferences, I plan to add an extra searchbox to diff.
My 2ct: This feature seems to be pretty related to the "Find" function - particularly to "Find all" in IDEs.
I would add it to the FindAndReplaceForm (or else have a new one opened using Shift+Ctrl+F). "Find & highlight all" does not work (and does not really apply) if invoked from the FileStatusList. So this button could be replaced with "Find all (git grep)" in this case.
The result as diff group feels appropriate.
There should be a way to discard the grep group.
I guess I would use it more often if the search could be limited to the changes instead of searching the entire file tree of the selected commit (which can be useful in other cases).
The detection of "-e" does not work in all locations. I have tried to ignore the character case:
My 2ct: This feature seems to be pretty related to the "Find" function - particularly to "Find all" in IDEs.
It is similar to Find all, but only for the files in Git and only for the selected commit, you do not have to check out the commits.
"Find & highlight all" does not work (and does not really apply) if invoked from the FileStatusList.
"Find & highlight all" is only for current file, should be renamed (moved to context menu in fileviewer?).
I would add it to the FindAndReplaceForm (or else have a new one opened using
Shift+Ctrl+F). .... So this button could be replaced with "Find all (git grep)" in this case. The result as diff group feels appropriate. There should be a way to discard the grep group.
git-grep works quite different from current find, so a separate popup is probably better than a combined form - maybe a button to switch to the other form.
A context menu item and keyboard shortcut to clear the search maybe.
Shift+Ctrl+F would be the logical shortcut, but it will hide the "find file".
But I may prefer the searchbox anyway, you can search interactively, get feedback faster.
I guess I would use it more often if the search could be limited to the changes instead of searching the entire file tree of the selected commit (which can be useful in other cases).
git-grep cannot search the diff, that would be a completely different algorithm. It could be possible to combine diff/grep, but the result would maybe be unexpected.
The detection of "-e" does not work in all locations. I have tried to ignore the character case:
Will try to tweak this. The options could be provided separately too. (But I do not want to implement all options in a gui, including and/or, overkill.)
But I may prefer the searchbox anyway, you can search interactively, get feedback faster. git-grep cannot search the diff, that would be a completely different algorithm.
I prefer code browsing in an IDE ("go to definition", "browse references", "exclude unit tests"). So I guess I will use grep rarely and rather save the space in favor of the Git Output Control. A toolbar button could be added to the Diff search bar in order to switch the toolbar mode to grep or else to show & hide an additional grep bar.
Shift+Ctrl+Fwould be the logical shortcut, but it will hide the "find file".
I think this is acceptable for the Diff tab - especially as grep is kind of a generalization of "find file". And one can quickly switch to the File Tree.
Implemented a searchbox. I believe this ready for review and inclusion, but there are some discussion points:
- mstv wanted to search with a popup instead. That could be added in addition to the searchbox, the box could be hidden.
- There are no tests. Will adapt FileStatusListTest.cs if this is accepted, but the tests depends on how it is implemented.
- If #11431 is merged, I prefer to add the methods in GiTModule to some other module (it is shared FileStatusList, GitUIExtensions and GrepHighlightService.
- documentation
Will squash if mstv agrees
Will squash if mstv agrees
I have not looked into the code yet.
If you have not reviewed the previous code, I assume I can squash.
mstv wanted to search with a popup instead. That could be added in addition to the searchbox, the box could be hidden.
My main point is that I do not want this searchbox to consume space all the time because I will rarely use it. I admit it is really quick and it can be (mis-)used to get all files for the artificial commits (by searching for ".").
Could we add a toggle button right of the
Filter filesbox? Though it would need to remain visible if there are "No changes".
I rather have an enable toggle in the context menu as for Show file differences (Ctrl-shift-f could be shortcut)
popup would be a way to configure the options though, maybe additional control.
I love the feature but the double dropdown doesn't look good...
As an idea, maybe we could have a single dropdown (visually, there could be several swapped in and out of the view) for the search, as we currently have, and another one to the right of it which allows picking the mode - i.e., the normal search or grep search?
E.g.:
As an idea, maybe we could have a single dropdown
It is useful to filter grep files too, grep and filter are not exclusive.
The POC I had initially separating with :: was better this way...
Maybe the popup is needed. I still like the extra search bar, may want to keep it as an option.
Edit: Would a hidden feature searching with :: in filter be acceptable for an initial merge, keeping as a hidden feature, to get some way of feedback?
Having it in one line with "::" feels too cryptic to me.
A toggle button in the upper right corner would make it discoverable. (Please use RuntimeSetting in order to not interfere with other running instances.)
Default search options could be set using a context menu (to be added in a follow-up).
In case of "no changes", I would always hide the search bar - but not change the setting.
Having it in one line with "::" feels too cryptic to me. A toggle button in the upper right corner would make it discoverable. (Please use
RuntimeSettingin order to not interfere with other running instances.) Default search options could be set using a context menu (to be added in a follow-up).
I am adding a configuration, default off, maybe even a popup. Depends on #11472 though, setting this to draft again.
I want the options to be persistent though, not just runtime. The filter items are instance local, could be be runtime settings.
In case of "no changes", I would always hide the search bar - but not change the setting.
Yes, in the same way as the filterbox is hidden if no changes.
I want the options to be persistent though, not just runtime. The filter items are instance local, could be be runtime settings.
That's what RuntimeSetting<> is intended for:
- no side-effect on other instances
- default persisted by means of
- discoverable with default value
true - insert below of "Find"?
- will need a description how to hide it by default
In case of "no changes", I would always hide the search bar - but not change the setting.
Yes, in the same way as the filterbox is hidden if no changes.
Not exactly, the search box still makes sense for empty commits. The user should be able to activate it.
If the "search commit" combobox is visible, the "filter files" combobox should always be shown, too. "No changes" shall be hidden, if there are search results.
I've incorporated this branch into my custom build and have already made use of it at least 3 times. Loving it! At the same time I am getting strong vibes that GitExtensions is turning in the kind of software where one needs to watch a youtube video or someone else using it to learn of these power-user features. Regardless, I still welcome this feature!
Pushed an update. This PR is now dependent on #11590, it uses the Git coloring. But I believe this PR works as I intend it too. I do not plan to change this PR much more but a follow up should add tests and documentation. A popup to search in addition to the searchbox in revdiff should be added too, also to set options.
Implemented a popup to search in (with the searchbox optional). Pushed to tmp/git-coloring. If #11590 is progressing, I will update this PR, including screenshots.
Doesn't look so good in 200% DPI
Zoomed in and marked-up
The grey placeholder text label problem (oversized) was there before too, but that didn't manifest visibly until this change.
The grey placeholder text label problem (oversized) was there before too, but that didn't manifest visibly until this change.
The label padding must be changed from 4->2 (but 15+2*4=23 so why is padding counted twice?) (OK with 100% too)
For the extra space I do not know, toggling the box mostly fixes the appearance (except for a smaller margin to the right).
Also the settings page
Fixing
I've manged to fix the label in my build by reverting these lines from a pretty recent PR https://github.com/gitextensions/gitextensions/pull/11472/files#diff-5cb23b1d441e66ddc2a3b26759cf7c0119dcf370010baff9cb2654611bdd891aR121-R124
I've manged to fix the label in my build by reverting these lines from a pretty recent PR https://github.com/gitextensions/gitextensions/pull/11472/files#diff-5cb23b1d441e66ddc2a3b26759cf7c0119dcf370010baff9cb2654611bdd891aR121-R124
That had other issues I believe. Fix (as well for settings) in tmp/git-coloring for now as I await that PR to be approved before updating this dependent PR again
Just wanted to add my 0.02€ that green color for highlighting looked much better (seen it when using older versions of this feature) as it gave out positive vibes of things found. Red color on the contrary gives errory vibes that things were removed or are missing.
Just wanted to add my 0.02€ that green color for highlighting looked much better (seen it when using older versions of this feature) as it gave out positive vibes of things found. Red color on the contrary gives errory vibes that things were removed or are missing.
I prefer green too, but Git uses red. Just bold by default, using the reverse red extra GE "theme" color as for the diff. (reverse color is used by many editors so that is not really a difference.)
Pushed a hack for the gap between the search and filter boxes. There is a minor display issue still if the search box is shown after startup, fixed with a toggle.
Another fix is to not persist the searchbox visibility after startup...
The update rebases the changes for #11590 too (that PR itself is not rebased).
Are you also getting this error upon startup in gerhardol/feature/i4912-git-grep-ui?
I've even did a git clean -xdf to make sure that it's not my local artifacts causing this.
Are you also getting this error upon startup in
gerhardol/feature/i4912-git-grep-ui? I've even did agit clean -xdfto make sure that it's not my local artifacts causing this.
I have seen it occasionally, not right now. (A recent PR should have handled that.)
Are you also getting this error upon startup in
gerhardol/feature/i4912-git-grep-ui? I've even did agit clean -xdfto make sure that it's not my local artifacts causing this.
A similar issue was fixed in #11533 @mstv any clues?
An alternative to 6050919c63b0c3bc220fe60fd516656305fa7c7a would be to not toggle search off/on while loading the control, as the size is apparently incorrect at that point. This would require that 8 occurrences that will never use the searchbox explicitly disables it or that the constructor gets an optional argument (the designer have problems with that?)
It is similar to #11533 but not that issue which has been addressed by it. The highlighted error message gives the clue. (I have not seen it myself.) The full callstack will make it clearer.