Skip to content

Conversation

mstv
Copy link
Member

@mstv mstv commented Jan 18, 2025

Proposed changes

RevisionDiffControl if folder selected:

  • Display names of contained items (with relative paths)
  • Enable actions on contained items

FileStatusList:

  • Add toolbar
  • Align git-grep and filter boxes

Screenshots

Before

image

image

After

image

image

image

Test methodology

  • manual
  • extended tests

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.

@mstv mstv self-assigned this Jan 18, 2025
@mstv mstv marked this pull request as draft January 18, 2025 11:44
@gerhardol
Copy link
Member

A few comments on usage:

  • Tooltip covers the text
    image
  • When is Dense tree/Group nodes not disabled? Is dropdown needed?
  • In some situations the list is not updated when clicking Group and/or !AB=. This occurs when selecting 421d22f and some situation with commits in two branches.

Noted, probably OK:

  • If a folder does not exist on disk "Show in folder" is disabled, as files are. Good. If top level group or status/extension status is shown it is available, opening the repo. Maybe change text to "Show repo folder"?
  • (maybe not this PR) If selecting a folder, "Save as" and others are available. This should maybe select the files in the folder too.

Some enhancements, probably future PRs:

  • Add File tree group, similar to the grep group. The file tree is improved in an update, but then it would not be needed.
  • Refresh button, for artificial commits. That can later show the status from GitStatusMonitor if the contents have changed.

@mstv
Copy link
Member Author

mstv commented Jan 19, 2025

Tooltip covers the text

Yes, not nice. It disappears on mouse move. Closing the dropdown does not work either. Blame WF.
Solution: Use ToolStripEx as in FormBrowse.

When is Dense tree/Group nodes not disabled? Is dropdown needed?

  • not implemented
  • when someone insists on
    • having a separate folder node for single-file folders
    • omitting group-by nodes (perhaps, only for flat list)

In some situations the list is not updated when clicking Group and/or !AB=.
This occurs when selecting 421d22f and some situation with commits in two branches.

If there are modified .cs files only, there is just one group - and hence no difference because singular group-by folder nodes are omitted.

Noted, probably OK:

If a folder does not exist on disk "Show in folder" is disabled, as files are. Good. If top level group or status/extension status is shown it is available, opening the repo. Maybe change text to "Show repo folder"?

I do not like to exchange menu item texts on runtime unless really necessary. I think opening the repo folder for root / group-by nodes is not counter-intuitive.

(maybe not this PR) If selecting a folder, "Save as" and others are available. This should maybe select the files in the folder too.

If you mean "Show in folder": A folder is selected. So I think, that this folder shall be selected in the Explorer.
Selecting multiple files opens multiple Explorer windows. If users really want all files to be shown in Explorer (windows), they should explicitly use "Select all" first.

Some enhancements, probably future PRs:

Add File tree group, similar to the grep group. The file tree is improved in an update, but then it would not be needed.

File tree needs slightly different handling. E.g. grouping does not really apply. I prefer to not add more groups, especially no expensive ones. It would need to be excluded from "find text in files".

Refresh button, for artificial commits. That can later show the status from GitStatusMonitor if the contents have changed.

👍 for those who do not use auto-refresh

image

I am going to add a split button with this setting, too.

GitStatusMonitor status to be added in a follow-up
BTW: Even with f0b5a24, GitStatusMonitor updates can be pretty slow - particularly with an AV virus.

@gerhardol
Copy link
Member

When is Dense tree/Group nodes not disabled? Is dropdown needed?

* not implemented

Remove from PR?

Noted, probably OK:
If a folder does not exist on disk "Show in folder" is disabled, as files are. Good. If top level group or status/extension status is shown it is available, opening the repo. Maybe change text to "Show repo folder"?

I do not like to exchange menu item texts on runtime unless really necessary. I think opening the repo folder for root / group-by nodes is not counter-intuitive.

It was not what I expected. No objection for now.

(maybe not this PR) If selecting a folder, "Save as" and others are available. This should maybe select the files in the folder too.

If you mean "Show in folder": A folder is selected. So I think, that this folder shall be selected in the Explorer. Selecting multiple files opens multiple Explorer windows. If users really want all files to be shown in Explorer (windows), they should explicitly use "Select all" first.

OK

Add File tree group, similar to the grep group. The file tree is improved in an update, but then it would not be needed.

File tree needs slightly different handling. E.g. grouping does not really apply. I prefer to not add more groups, especially no expensive ones. It would need to be excluded from "find text in files".

This group should only be calculated when expanded, we leave it here for now.

Refresh button, for artificial commits. That can later show the status from GitStatusMonitor if the contents have changed.

👍 for those who do not use auto-refresh

Auto refresh is too slow for me in some repos

I am going to add a split button with this setting, too.

GitStatusMonitor status to be added in a follow-up BTW: Even with f0b5a24, GitStatusMonitor updates can be pretty slow - particularly with an AV virus.

I would have preferred to have it leftmost as for the main toolbar and being disabled if no artificial. Now it is moving places...

@mstv
Copy link
Member Author

mstv commented Jan 20, 2025

leftmost as for the main toolbar and being disabled if no artificial. Now it is moving places...

static position now, but the Collapse button is more important

image

can later show the status from GitStatusMonitor if the contents have changed

Currently, this essential information is not displayed (or not retrieved). The numbers at the commit button and at the artificial commits do not really help in indicating whether a refresh is needed or not.
This toolbar must not contain text. Text is very distracting here. Dynamic texts would be even worse.
image "ReloadRevisionsDirty.png" should be used in order to indicate that a refresh is required.

@gerhardol
Copy link
Member

Currently, this essential information is not displayed (or not retrieved). The numbers at the commit button and at the artificial commits do not really help in indicating whether a refresh is needed or not.

It would have to compare current status with the update, if ever implemented.

This toolbar must not contain text. Text is very distracting here. Dynamic texts would be even worse.

Agree, numbers are in the group (may be different) and grid.

@mstv mstv force-pushed the feature/filestatuslist_toolbar branch 2 times, most recently from d0ba2e6 to cebdb3a Compare January 22, 2025 20:09
mstv added a commit to mstv/gitextensions that referenced this pull request Jan 30, 2025
mstv added a commit to mstv/gitextensions that referenced this pull request Jan 30, 2025
mstv added a commit to mstv/gitextensions that referenced this pull request Jan 30, 2025
mstv added a commit to mstv/gitextensions that referenced this pull request Jan 30, 2025
mstv added a commit to mstv/gitextensions that referenced this pull request Jan 30, 2025
mstv added a commit to mstv/gitextensions that referenced this pull request Jan 30, 2025
mstv added a commit to mstv/gitextensions that referenced this pull request Jan 30, 2025
mstv added a commit to mstv/gitextensions that referenced this pull request Jan 30, 2025
mstv added a commit to mstv/gitextensions that referenced this pull request Jan 30, 2025
mstv added a commit to mstv/gitextensions that referenced this pull request Jan 30, 2025
@mstv mstv force-pushed the feature/filestatuslist_toolbar branch from cebdb3a to 462593f Compare January 30, 2025 20:43
mstv added a commit to mstv/gitextensions that referenced this pull request Jan 30, 2025
mstv added a commit to mstv/gitextensions that referenced this pull request Jan 30, 2025
mstv added a commit to mstv/gitextensions that referenced this pull request Jan 30, 2025
mstv added a commit to mstv/gitextensions that referenced this pull request Jan 30, 2025
mstv added a commit to mstv/gitextensions that referenced this pull request Jan 30, 2025
@mstv mstv force-pushed the feature/filestatuslist_toolbar branch from 462593f to 216c28c Compare January 30, 2025 22:00
@mstv mstv marked this pull request as ready for review February 3, 2025 21:47
mstv added a commit to mstv/gitextensions that referenced this pull request Mar 25, 2025
mstv added a commit to mstv/gitextensions that referenced this pull request Mar 25, 2025
mstv added a commit to mstv/gitextensions that referenced this pull request Mar 25, 2025
mstv added a commit to mstv/gitextensions that referenced this pull request Mar 25, 2025
mstv added a commit to mstv/gitextensions that referenced this pull request Mar 25, 2025
if it is a single item (i.e. a leaf node)

Refs: gitextensions#12149
@mstv mstv force-pushed the feature/filestatuslist_toolbar branch from 66586cd to ff95d37 Compare March 25, 2025 20:21
mstv added a commit to mstv/gitextensions that referenced this pull request Mar 26, 2025
mstv added a commit to mstv/gitextensions that referenced this pull request Mar 26, 2025
mstv added a commit to mstv/gitextensions that referenced this pull request Mar 26, 2025
if it is a single item (i.e. a leaf node)

Refs: gitextensions#12149
@mstv mstv force-pushed the feature/filestatuslist_toolbar branch from ff95d37 to 3762e9d Compare March 26, 2025 20:45
@mstv
Copy link
Member Author

mstv commented Mar 26, 2025

Sorry, yet another two addenda

@mstv mstv force-pushed the feature/filestatuslist_toolbar branch from 3762e9d to 43dbda3 Compare March 31, 2025 21:01
@mstv mstv merged commit 43dbda3 into gitextensions:master Mar 31, 2025
4 checks passed
@mdonatas
Copy link
Contributor

mdonatas commented Apr 1, 2025

Huge effort, congratulations! 🚀

@mstv mstv deleted the feature/filestatuslist_toolbar branch April 1, 2025 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants