Expose git-grep in settings and context menu #11858

RussKie · 2024-08-27T09:52:22Z

Addendum to #11350

Proposed changes

Expose "git-grep" search in Settings
Expose "git-grep" search in the context menu

Screenshots

Before

After

Merge strategy

I agree that the maintainer squash merge this PR (if the commit message is clear).

✒️ I contribute this code under The Developer Certificate of Origin.

gerhardol · 2024-08-27T13:12:26Z

Git grep searchbox was somehow hidden on your request, you found it confusing.
To search from the popup, there is a context menu. This is similar to find in diff, Ctrl-F.

You can enable the searchbox in the popup, not much in the help though...

I do not oppose a more direct configuration of the searchbox, but the names are not aligned. Search commit could be renamed too, maybe git-grep should be mentioned at least.

I do not

gerhardol

Is this really needed if the popup is the primary way to use git-grep?

mstv · 2024-08-27T15:45:07Z

My 2ct:

Having the option in the settings dialog might be useful and will be discoverable.
The second context menu item is too redundant to me.
The term "Search files in commit (git-grep)" could be used thoroughly.

gerhardol · 2024-08-27T21:22:38Z

My 2ct:

Fine for me

RussKie · 2024-08-28T00:25:53Z

The term "Search files in commit (git-grep)" could be used thoroughly.

I wasn't sure on the wording we wanted to use; this is a good suggestion.

The second context menu item is too redundant to me.

The reason I added it was without it a user has to open the popup to enable/disable the git-grep search box. That's not a good UX.

Is this really needed if the popup is the primary way to use git-grep?

I believe there is - the popup allows to specify additional git-grep option, whilst the search box doesn't.

Git grep searchbox was somehow hidden on your request, you found it confusing.

Yes, I remember that, and I still think having two of those visible by default isn't great. But needing to open another dialog just to show the search box is equally not a good UX.

RussKie · 2024-08-28T00:45:09Z

Not exactly related, but can you help me understand why we have mnemonics in the context menu? The contextmenu items can't be invoked via the mnemonics, only via the shortcuts. ...or am I missing something?

Not to mention we have conflicting shortcuts and mnemonics:

mstv · 2024-08-28T05:16:10Z

can you help me understand why we have mnemonics in the context menu? The contextmenu items can't be invoked via the mnemonics, only via the shortcuts. ...or am I missing something?

There is a dedicated key for opening the context menu (right of the space bar). Shift+F10 opens it, too.
Right click + letter is much quicker than aiming with the mouse (at least to me).

RussKie · 2024-08-28T10:16:34Z

There is a dedicated key for opening the context menu (right of the space bar). Shift+F10 opens it, too.

My keyboard doesn't have a dedicated button, though I think I saw such keyboards.
Thank you for the Shift+F10 tip (though I feel like I should have known about it).

2. Right click + letter is much quicker than aiming with the mouse (at least to me).

:-O

mstv · 2024-08-28T17:22:50Z

Yet another text to be aligned:

And isn't it a "find box" then, too?

gerhardol · 2024-08-28T21:55:33Z

I rather use SearchCommit or "SearchCommit (git-grep)" in UI and SearchCommitGitGrep in code.
While git-grep refers to the Git functionality, I have got the feedback that SearchCommit is a better description.
There is already "Find file" in both RevDiff and RevFileTree, that have different functionality.

The search box was considered to be an advanced usage (but I use it daily) why the RevDiff context menu may be redundant. But it can stay if the name is aligned with the invocation of the popup.

gerhardol · 2024-08-28T21:57:40Z

src/app/GitUI/UserControls/FileStatusList.cs


-        private void EnableSearchForList(bool enable)
+        public void ToggleGitGrepSearch(bool visible)


Toggle is not a good name, it updates the status at every update.
I have some doubts for some of the changes, not time to investigate today....

it updates the status at every update.

Which "status" are you talking about?

RussKie · 2024-08-29T01:24:58Z

I rather use SearchCommit or "SearchCommit (git-grep)" in UI and SearchCommitGitGrep in code.

Sounds good, I'll change

RussKie · 2024-08-29T01:26:16Z

Yet another text to be aligned:

And isn't it a "find box" then, too?

Missed that text, thanks

RussKie · 2024-08-29T08:22:47Z

gerhardol · 2024-08-29T08:53:03Z

My preference is still Search commit (git-grep).
Also, it is not a filter for Diff (but the commit). So the settings/menu/popup-box could be Show 'Search commit (git-grep)' box
Box text has nothing about regex, maybe not needed, I consider 'files' less important: Search commit files with git-grep -> Search commit (git-grep)

gerhardol

+1 One question on the fuctionality and suggestion to change UI text slightly, otherwise fine.

src/app/GitUI/UserControls/FileStatusList.cs

RussKie · 2024-09-02T07:26:47Z

Updated

mstv

If you are too busy, I can commit the proposals.

The current state is too inconsistent to me:

mstv · 2024-09-02T17:32:13Z

src/app/GitUI/CommandsDialogs/RevisionDiffControl.Designer.cs

+            showSearchCommitGitGrepDialogToolStripMenuItem.Image = Properties.Images.ViewFile;
+            showSearchCommitGitGrepDialogToolStripMenuItem.Name = "showSearchCommitGitGrepDialogToolStripMenuItem";
+            showSearchCommitGitGrepDialogToolStripMenuItem.Size = new Size(262, 22);
+            showSearchCommitGitGrepDialogToolStripMenuItem.Text = "Search commit files with git-grep...";


This function does not "find files" but "searches for text in files of the commit's tree".
That's why we agreed to "search files in commit" in the initial PR.

Suggested change

showSearchCommitGitGrepDialogToolStripMenuItem.Text = "Search commit files with git-grep...";

showSearchCommitGitGrepDialogToolStripMenuItem.Text = "Sear&ch files in commit with git-grep...";

I think "Search commit (git-grep)" is fine too, slightly shorter.

In general, I completely disagree with the chosen wording.
We have an existing form and wording for searching in files:

I was aligning the wording with that to provide consistent UX. Now, we have "File file.." and "Search files..." - what's the difference? Both search content in commit files. Different algorithms, but ultimately the same operation - search in files.

The suggested wording is generally different to what other apps use - e.g., VS:

I was aligning the wording with that to provide consistent UX. Now, we have "File file.." and "Search files..." - what's the difference? Both search content in commit files. Different algorithms, but ultimately the same operation - search in files.

Find is searching the files in the diff
Search commit is searching the files in the commit

It is really the existing Find that is different from "search in working directory" for search-all apps.
But changing functionality may confuse users. Maybe that test should be changed though.
regardless, I want commit in the label to differentiate this addition. "Find files in commit (git-grep)" is longer than "Search commit (git-grep)". (having git-grep in the label is a bonus)

Will this work?

I would like to have commit in the name
Find file is not a fileter (it would be if in the file-tree)

So the highlighted names (similar for popup), my suggestions

Find in files for commit (git-grep) with regular expression
Find in files for commit (git-grep)...
Show 'Find in files for commit (git-grep)' box

@gerhardol sounds good.
@mstv would it work for you?

Yes, this will work for me.

I pushed the changes and updated the screenshots in the first post.

I kept the text in watermark a) for consistency reasons and b) the context here is obvious - the commit - so the word is redundant.

This drove the wording in the context menu, the Settings and in the Find dialog.

src/app/GitUI/CommandsDialogs/RevisionDiffControl.Designer.cs

src/app/GitUI/CommandsDialogs/RevisionDiffControl.cs

src/app/GitCommands/Settings/AppSettings.cs

src/app/GitUI/CommandsDialogs/RevisionDiffControl.cs

src/app/GitUI/Editor/FormFindFileGitGrepCommit.Designer.cs

src/app/GitUI/UserControls/FileStatusList.cs

mstv

I buy the wording.

src/app/GitCommands/Settings/AppSettings.cs

src/app/GitUI/CommandsDialogs/RevisionDiffControl.Designer.cs

mstv · 2024-09-10T19:45:45Z

src/app/GitUI/CommandsDialogs/RevisionDiffControl.Designer.cs

+            showFindInCommitFilesGitGrepFilterToolStripMenuItem.CheckOnClick = true;
+            showFindInCommitFilesGitGrepFilterToolStripMenuItem.Name = "showFindInCommitFilesGitGrepFilterToolStripMenuItem";
+            showFindInCommitFilesGitGrepFilterToolStripMenuItem.Size = new Size(262, 22);
+            showFindInCommitFilesGitGrepFilterToolStripMenuItem.Text = "Show 'Filter files using git-grep regular expression'";


Suggested change

showFindInCommitFilesGitGrepFilterToolStripMenuItem.Text = "Show 'Filter files using git-grep regular expression'";

showFindInCommitFilesGitGrepFilterToolStripMenuItem.Text = "Show 'Fi&lter files using git-grep regular expression'";

Show 'Find files in commit using git-grep regular expression'";

No, calling it "Find files.." makes it inconsistent both visually and conceptually.

Both controls have consistent wording:

And conceptually both of these controls filter the list of files - one filters by file name and the other filters by content. By both perform the same kind of operation - filter the list of files.

Also, there's a semantical difference between "find" and "filter". The latter reduces a given list based on criteria, whilst the former locates requested items. And this control is about reducing the list of commit files in the UI based on a search string.

🤔 Perhaps the watermark in the second control should be reworded to "Filter files by name using regul...."

src/app/GitUI/CommandsDialogs/RevisionDiffControl.cs

src/app/GitUI/CommandsDialogs/SettingsDialog/Pages/FormBrowseRepoSettingsPage.Designer.cs

mstv · 2024-09-10T19:57:47Z

src/app/GitUI/Editor/FormInCommitFilesGitGrep.Designer.cs

@@ -3,12 +3,12 @@

 namespace GitUI
 {
-	partial class FormSearchCommit
+    partial class FormFindInCommitFilesGitGrep


"Find" is missing in the file name.
Though as we use "git-grep" in the UI, it should suffice to use just "GitGrep" internally, i.e.

Suggested change

partial class FormFindInCommitFilesGitGrep

partial class FormGitGrep

Similar reasoning as in #11858 (comment) - this rename won't add any value but will increase inconsistency, which will make it harder to build a mental picture when working with the codebase.

mstv · 2024-09-10T20:01:28Z

src/app/GitUI/UserControls/FileStatusList.cs

        private Rectangle _dragBoxFromMouseDown;
        private IDisposable? _selectedIndexChangeSubscription;
        private IDisposable? _diffListSortSubscription;
-        private FormSearchCommit _formSearchCommit;
+        private FormFindInCommitFilesGitGrep _formFindFileGitGrepCommit;


Suggested change

private FormFindInCommitFilesGitGrep _formFindFileGitGrepCommit;

private FormFindInCommitFilesGitGrep _formGitGrep;

src/app/GitUI/UserControls/FileStatusList.cs

gerhardol · 2024-09-10T21:41:45Z

src/app/GitUI/CommandsDialogs/RevisionDiffControl.Designer.cs

+            showFindInCommitFilesGitGrepFilterToolStripMenuItem.CheckOnClick = true;
+            showFindInCommitFilesGitGrepFilterToolStripMenuItem.Name = "showFindInCommitFilesGitGrepFilterToolStripMenuItem";
+            showFindInCommitFilesGitGrepFilterToolStripMenuItem.Size = new Size(262, 22);
+            showFindInCommitFilesGitGrepFilterToolStripMenuItem.Text = "Show 'Filter files using git-grep regular expression'";


Show 'Find files in commit using git-grep regular expression'";

src/app/GitUI/Editor/FormInCommitFilesGitGrep.Designer.cs

src/app/GitUI/UserControls/FileStatusList.Designer.cs

gerhardol

Pushed an update, aligning the names

FindInCommitFilesGitGrep in names (not Using)
Find in commit files using git-grep in texts

It would had been easier if we aligned on names before the changes (even better when handling the PR, but that will not always happen).

mstv

The "Show '...'" checkbox should be left aligned.

src/app/GitUI/CommandsDialogs/RevisionDiffControl.Designer.cs

src/app/GitUI/CommandsDialogs/SettingsDialog/Pages/FormBrowseRepoSettingsPage.Designer.cs

mstv · 2024-09-11T20:36:17Z

src/app/GitUI/Editor/FormFindInCommitFilesGitGrep.Designer.cs

            // 
            AcceptButton = btnSearch;
            AutoScaleDimensions = new SizeF(96F, 96F);
            AutoScaleMode = AutoScaleMode.Dpi;
-            ClientSize = new Size(419, 144);
+            ClientSize = new Size(425, 144);


Height needs to be adapted.

shortened the text. This could still be an issue in some translations (French and German for instance)
This is positioned automatically and will not go further left

src/app/GitUI/UserControls/FileStatusList.Designer.cs

src/app/GitUI/UserControls/FileStatusList.cs

gerhardol · 2024-09-18T19:55:14Z

mstv

Trivial MC (adjacent changes) to be resolved yet.

gerhardol · 2024-09-18T20:34:11Z

Trivial MC (adjacent changes) to be resolved yet.

Since you have reviewed, I will squash and rebase tomorrow unless @RussKie protests

gerhardol · 2024-09-18T20:36:39Z

Additional: I thought we discussed naming a lot for the original PR before we settled on something.
However, the additional discussions and changes back and forward in this PR has required even more time.
Naming is hard.

RussKie · 2024-09-20T05:05:51Z

Additional: I thought we discussed naming a lot for the original PR before we settled on something. However, the additional discussions and changes back and forward in this PR has required even more time. Naming is hard.

We did but some features are hard to appreciate without actively using those first. Naming and UI are hard indeed.

gerhardol · 2024-09-20T18:19:53Z

@mstv Do you want to add your latest tweaks too?

mstv · 2024-09-20T19:27:56Z

@mstv Do you want to add your latest tweaks too?

I hesitated but a774285 and a774285 belong to this PR - pushing.
The third one is worth a separate PR.

gerhardol · 2024-09-20T20:45:17Z

src/app/GitUI/CommandsDialogs/RevisionDiffControl.Designer.cs

-            showFindInCommitFilesGitGrepToolStripMenuItem,
-            toolStripSeparatorScript,
-            runScriptToolStripMenuItem});
+            DiffContextMenu.Items.AddRange(new ToolStripItem[] { diffUpdateSubmoduleMenuItem, diffResetSubmoduleChanges, diffStashSubmoduleChangesToolStripMenuItem, diffCommitSubmoduleChanges, submoduleStripSeparator, stageFileToolStripMenuItem, unstageFileToolStripMenuItem, resetFileToToolStripMenuItem, cherryPickSelectedDiffFileToolStripMenuItem, toolStripSeparator32, openWithDifftoolToolStripMenuItem, diffOpenWorkingDirectoryFileWithToolStripMenuItem, diffOpenRevisionFileToolStripMenuItem, diffOpenRevisionFileWithToolStripMenuItem, saveAsToolStripMenuItem1, diffEditWorkingDirectoryFileToolStripMenuItem, diffDeleteFileToolStripMenuItem, diffToolStripSeparator13, copyPathsToolStripMenuItem, openContainingFolderToolStripMenuItem, toolStripSeparator33, diffShowInFileTreeToolStripMenuItem, diffFilterFileInGridToolStripMenuItem, fileHistoryDiffToolstripMenuItem, blameToolStripMenuItem, findInDiffToolStripMenuItem, showFindInCommitFilesGitGrepDialogToolStripMenuItem, showFindInCommitFilesGitGrepToolStripMenuItem, toolStripSeparatorScript, runScriptToolStripMenuItem });


Why is the format as this?
I can use difftastic to see that the order is the same but it seem unneeded.

Otherwise no comment on the changes

Why is the format as this?

As mentioned in the commit message "chore(RevisionDiffControl): Saved without changes using Designer 17.11.3".
Don't ask me!

I prefer to revert this change

I prefer to revert this change

The next time someone needs to use the designer, the problem will pop up again.
And it already has: I have already had a hard evening rebasing and merging designer files because pull request reviews are not finished or do not happen.

reverted because there are is another nonsense change by the Designer to be manually reverted.

Any further objections / vetos, @gitextensions/git-extensions-team?

Align names: FindInCommitFilesGitGrep in names (not Using) Find in commit files using git-grep in texts Refs: gitextensions#11858, addendum to gitextensions#11350

using Designer 17.11.3 Refs: gitextensions#11858

RussKie requested review from gerhardol and mstv August 27, 2024 09:52

RussKie added this to the 5.0.0 milestone Aug 27, 2024

gerhardol reviewed Aug 27, 2024

View reviewed changes

gerhardol reviewed Aug 28, 2024

View reviewed changes

gerhardol reviewed Aug 29, 2024

View reviewed changes

src/app/GitUI/UserControls/FileStatusList.cs Outdated Show resolved Hide resolved

src/app/GitUI/UserControls/FileStatusList.cs Show resolved Hide resolved

src/app/GitUI/UserControls/FileStatusList.cs Show resolved Hide resolved

RussKie force-pushed the gitgrep branch from ad92da2 to a2a99a1 Compare September 2, 2024 07:19

mstv reviewed Sep 2, 2024

View reviewed changes

RussKie modified the milestones: 5.0.0, 5.0.1 Sep 9, 2024

RussKie force-pushed the gitgrep branch from b32a337 to e69157d Compare September 10, 2024 01:59

mstv reviewed Sep 10, 2024

View reviewed changes

gerhardol reviewed Sep 10, 2024

View reviewed changes

gerhardol approved these changes Sep 11, 2024

View reviewed changes

mstv reviewed Sep 11, 2024

View reviewed changes

mstv approved these changes Sep 18, 2024

View reviewed changes

gerhardol force-pushed the gitgrep branch from 76cf4a8 to bd978e0 Compare September 19, 2024 19:06

gerhardol reviewed Sep 20, 2024

View reviewed changes

mstv force-pushed the gitgrep branch from 657092d to 7b19cd9 Compare September 27, 2024 20:55

RussKie and others added 2 commits September 27, 2024 22:56

fix: Expose git-grep box in settings and context menu

1292e52

Align names: FindInCommitFilesGitGrep in names (not Using) Find in commit files using git-grep in texts Refs: gitextensions#11858, addendum to gitextensions#11350

chore(RevisionDiffControl): Saved without changes

ad4bce2

using Designer 17.11.3 Refs: gitextensions#11858

mstv force-pushed the gitgrep branch from 7b19cd9 to ad4bce2 Compare September 27, 2024 20:57

mstv merged commit ad4bce2 into gitextensions:master Sep 27, 2024
2 of 4 checks passed

mstv mentioned this pull request Oct 7, 2024

fix(ReportDubiousOwnership): Support normal paths, too #11965

Merged

pmiossec mentioned this pull request Nov 7, 2024

[NBug] Invoke or BeginInvoke cannot be called on a control until t... #12026

Closed

RussKie deleted the gitgrep branch April 5, 2025 08:27


		private void EnableSearchForList(bool enable)
		public void ToggleGitGrepSearch(bool visible)

	showSearchCommitGitGrepDialogToolStripMenuItem.Text = "Search commit files with git-grep...";
	showSearchCommitGitGrepDialogToolStripMenuItem.Text = "Sear&ch files in commit with git-grep...";

	showFindInCommitFilesGitGrepFilterToolStripMenuItem.Text = "Show 'Filter files using git-grep regular expression'";
	showFindInCommitFilesGitGrepFilterToolStripMenuItem.Text = "Show 'Fi&lter files using git-grep regular expression'";

	partial class FormFindInCommitFilesGitGrep
	partial class FormGitGrep

	private FormFindInCommitFilesGitGrep _formFindFileGitGrepCommit;
	private FormFindInCommitFilesGitGrep _formGitGrep;

Uh oh!

Expose git-grep in settings and context menu #11858

Expose git-grep in settings and context menu #11858

Uh oh!

Conversation

RussKie commented Aug 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Screenshots

Before

After

Merge strategy

Uh oh!

gerhardol commented Aug 27, 2024

Uh oh!

gerhardol left a comment

Choose a reason for hiding this comment

Uh oh!

mstv commented Aug 27, 2024

Uh oh!

gerhardol commented Aug 27, 2024

Uh oh!

RussKie commented Aug 28, 2024

Uh oh!

RussKie commented Aug 28, 2024

Uh oh!

mstv commented Aug 28, 2024

Uh oh!

RussKie commented Aug 28, 2024

Uh oh!

mstv commented Aug 28, 2024

Uh oh!

gerhardol commented Aug 28, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RussKie commented Aug 29, 2024

Uh oh!

RussKie commented Aug 29, 2024

Uh oh!

RussKie commented Aug 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gerhardol commented Aug 29, 2024

Uh oh!

gerhardol left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RussKie commented Sep 2, 2024

Uh oh!

mstv left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

RussKie commented Aug 27, 2024 •

edited

Loading

RussKie commented Aug 29, 2024 •

edited

Loading

mstv left a comment •

edited

Loading