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
Worktree: redesign 'Manage worktree' form #10224
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just nitpicks
A Create
button would be useful, too.
if (branch == null || !branch.StartsWith("refs/heads/")) | ||
{ | ||
return branch; | ||
} | ||
|
||
return branch.Substring(11); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (branch == null || !branch.StartsWith("refs/heads/")) | |
{ | |
return branch; | |
} | |
return branch.Substring(11); | |
if (branch is not null && branch.StartsWith(GitRefName.RefsHeadsPrefix)) | |
{ | |
return branch.SubString(GitRefName.RefsHeadsPrefix.Length); | |
} | |
return branch; |
@@ -112,53 +117,21 @@ private void Initialize() | |||
} | |||
|
|||
Worktrees.DataSource = _worktrees; | |||
|
|||
Font font = Worktrees.DefaultCellStyle.Font; | |||
var deletedFont = new Font(font.FontFamily, font.Size, font.Style | FontStyle.Strikeout); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
var deletedFont = new Font(font.FontFamily, font.Size, font.Style | FontStyle.Strikeout); | |
Font deletedFont = new(font.FontFamily, font.Size, font.Style | FontStyle.Strikeout); |
this.buttonPruneWorktrees.Name = "buttonPruneWorktrees"; | ||
this.buttonPruneWorktrees.Size = new System.Drawing.Size(189, 23); | ||
this.buttonPruneWorktrees.TabIndex = 1; | ||
this.buttonPruneWorktrees.Text = "Prune deleted worktrees"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.buttonPruneWorktrees.Text = "Prune deleted worktrees"; | |
this.buttonPruneWorktrees.Text = "&Prune deleted worktrees"; |
this.buttonDeleteSelectedWorktree.Name = "buttonDeleteSelectedWorktree"; | ||
this.buttonDeleteSelectedWorktree.Size = new System.Drawing.Size(189, 23); | ||
this.buttonDeleteSelectedWorktree.TabIndex = 1; | ||
this.buttonDeleteSelectedWorktree.Text = "Delete selected worktree"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.buttonDeleteSelectedWorktree.Text = "Delete selected worktree"; | |
this.buttonDeleteSelectedWorktree.Text = "&Delete selected worktree"; |
this.buttonOpenSelectedWorktree.Name = "buttonOpenSelectedWorktree"; | ||
this.buttonOpenSelectedWorktree.Size = new System.Drawing.Size(189, 23); | ||
this.buttonOpenSelectedWorktree.TabIndex = 1; | ||
this.buttonOpenSelectedWorktree.Text = "Open selected worktree"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.buttonOpenSelectedWorktree.Text = "Open selected worktree"; | |
this.buttonOpenSelectedWorktree.Text = "&Open selected worktree"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better, now it looks like a normal form.
have not run
A Create button would be useful, too.
Suggest that is only accessibly in the Manage form, so only one menu item in File menu
This affects translations, so I do not know if it is possible to include in 4.0
(This is not a high profile form though)
In fact this commit was a first step toward "merging" the manage and create forms because I noticed that nearly 100% of the times I want to see the existing worktrees before creating one. And as a consequence having only one menu item. But my first attempt was not working and I don't really get why... I will try to find some room for the create button and update the menu. |
47543f4
to
5b380b8
Compare
Done. (and screenshot updated) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be included in 4.0, even if translations are changed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for eliminating the submenu!
I have further wishes:
// buttonDeleteSelectedWorktree | ||
// | ||
this.buttonDeleteSelectedWorktree.Anchor = ((System.Windows.Forms.AnchorStyles)((System.Windows.Forms.AnchorStyles.Bottom | System.Windows.Forms.AnchorStyles.Left))); | ||
this.buttonDeleteSelectedWorktree.Image = global::GitUI.Properties.Images.DeleteFile; | ||
this.buttonDeleteSelectedWorktree.ImageAlign = System.Drawing.ContentAlignment.MiddleLeft; | ||
this.buttonDeleteSelectedWorktree.Location = new System.Drawing.Point(165, 226); | ||
this.buttonDeleteSelectedWorktree.Name = "buttonDeleteSelectedWorktree"; | ||
this.buttonDeleteSelectedWorktree.Size = new System.Drawing.Size(151, 23); | ||
this.buttonDeleteSelectedWorktree.TabIndex = 1; | ||
this.buttonDeleteSelectedWorktree.Text = "&Delete selected"; | ||
this.buttonDeleteSelectedWorktree.UseVisualStyleBackColor = true; | ||
this.buttonDeleteSelectedWorktree.Click += new System.EventHandler(this.buttonDeleteSelectedWorktree_Click); | ||
// | ||
// buttonOpenSelectedWorktree | ||
// | ||
this.buttonOpenSelectedWorktree.Anchor = ((System.Windows.Forms.AnchorStyles)((System.Windows.Forms.AnchorStyles.Bottom | System.Windows.Forms.AnchorStyles.Left))); | ||
this.buttonOpenSelectedWorktree.Image = global::GitUI.Properties.Images.BrowseFileExplorer; | ||
this.buttonOpenSelectedWorktree.ImageAlign = System.Drawing.ContentAlignment.MiddleLeft; | ||
this.buttonOpenSelectedWorktree.Location = new System.Drawing.Point(8, 226); | ||
this.buttonOpenSelectedWorktree.Name = "buttonOpenSelectedWorktree"; | ||
this.buttonOpenSelectedWorktree.Size = new System.Drawing.Size(151, 23); | ||
this.buttonOpenSelectedWorktree.TabIndex = 1; | ||
this.buttonOpenSelectedWorktree.Text = "&Open selected"; | ||
this.buttonOpenSelectedWorktree.UseVisualStyleBackColor = true; | ||
this.buttonOpenSelectedWorktree.Click += new System.EventHandler(this.buttonOpenSelectedWorktree_Click); | ||
// | ||
// buttonCreateNewWorktree | ||
// | ||
this.buttonCreateNewWorktree.Anchor = ((System.Windows.Forms.AnchorStyles)((System.Windows.Forms.AnchorStyles.Bottom | System.Windows.Forms.AnchorStyles.Left))); | ||
this.buttonCreateNewWorktree.Image = global::GitUI.Properties.Images.FileStatusAdded; | ||
this.buttonCreateNewWorktree.ImageAlign = System.Drawing.ContentAlignment.MiddleLeft; | ||
this.buttonCreateNewWorktree.Location = new System.Drawing.Point(322, 226); | ||
this.buttonCreateNewWorktree.Name = "buttonCreateNewWorktree"; | ||
this.buttonCreateNewWorktree.Size = new System.Drawing.Size(151, 23); | ||
this.buttonCreateNewWorktree.TabIndex = 1; | ||
this.buttonCreateNewWorktree.Text = "&Create..."; | ||
this.buttonCreateNewWorktree.UseVisualStyleBackColor = true; | ||
this.buttonCreateNewWorktree.Click += new System.EventHandler(this.buttonCreateNewWorktree_Click); | ||
// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would move these up in order to get a clear Tab order.
private void buttonCreateNewWorktree_Click(object sender, EventArgs e) | ||
{ | ||
SwitchToCreateWorktreeForm = true; | ||
DialogResult = DialogResult.Cancel; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather keep this dialog open and open the FormCreateWorktree directly.
The reaction to Open the new worktree after creation
can reuse buttonOpenSelectedWorktree
.
5b380b8
to
15061cf
Compare
@mstv Changes done. |
15061cf
to
114cd52
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
* Add "Open" and "Delete" buttons instead of having it in the grid * Strike line when a worktree is deleted (and so remove "Deleted" column) * Change font of Sha1 column & column width set to fill the remaining space * branch name displayed more user friendly (without '/refs/heads' prefix) * Leave only "Manage worktree..." entry from FormBrowse menu * Add a "Create..." button to open formCreateWorktree from the "Manage worktree" form & Refresh list of worktrees if user select to not open the newly worktree created
114cd52
to
f53fb6f
Compare
Squashed and rebased. I think that's ready to be merge... |
* Add "Open" and "Delete" buttons instead of having it in the grid * Strike line when a worktree is deleted (and so remove "Deleted" column) * Change font of Sha1 column & column width set to fill the remaining space * branch name displayed more user friendly (without '/refs/heads' prefix) * Leave only "Manage worktree..." entry from FormBrowse menu * Add a "Create..." button to open formCreateWorktree from the "Manage worktree" form & Refresh list of worktrees if user select to not open the newly worktree created (cherry picked from commit 5f2071f)
Proposed changes
Screenshots
Before
After
Test methodology
Test environment(s)
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.