Skip to content

FormResolveConflicts: Improve help when merging/rebasing #11619

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Mar 21, 2024
Merged

FormResolveConflicts: Improve help when merging/rebasing #11619

merged 4 commits into from
Mar 21, 2024

Conversation

pmiossec
Copy link
Member

@pmiossec pmiossec commented Mar 7, 2024 •
edited
Loading

Proposed changes

When conflicts during merge/rebase, it is not always easy to what is "local" or "remote" ( See here and here )

  • Add also terms "current"/"incoming" (that I found a little more explicit) and that user could be used to see if merging with "vscode"
  • Add more explicit help as tooltips

Screenshots

Before

  • Merge:

image

  • Rebase:

image

After

  • Merge:

image

  • Rebase:

image

Test methodology

  • Manual

Test environment(s)

  • Git Extensions 33.33.33
  • Build 199b993
  • Git 2.44.0.windows.1
  • Microsoft Windows NT 10.0.22631.0
  • .NET 8.0.1
  • DPI 96dpi (no scaling)
  • Portable: False

Merge strategy

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

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

@ghost ghost assigned pmiossec Mar 7, 2024
pmiossec added a commit to pmiossec/gitextensions that referenced this pull request Mar 11, 2024
@pmiossec
FormResolveConflicts: Improve help when merging/rebasing (gitextensio…
a2af092 
…ns#11619)
@pmiossec pmiossec force-pushed the conflicts_little_help branch from 199b993 to a2af092 Compare March 11, 2024 09:25
@gerhardol
Copy link
Member

I would prefer to have the branch name where available, then commit, then remote/incoming (keep ours/theirs as it is used by Git).
Adding incoming will make the text even longer.

@mstv
Copy link
Member

mstv commented Mar 14, 2024

👍 When rebasing, the term "incoming (ours)" is much clearer to me than just "remote (ours)".
"remote/" and "local/" help to identify the regarding files in the merge tool.

It will be most helpful to me to have these additions in the lower pane of the dialog (green arrows). This can be looked up without closing the merge tool.
The help text (yellow arrows) would not benefit very much. It even might become too long.

image

pmiossec added a commit to pmiossec/gitextensions that referenced this pull request Mar 19, 2024
@pmiossec
FormResolveConflicts: Improve help when merging/rebasing (gitextensio…
2e813d5 
…ns#11619)
pmiossec added a commit to pmiossec/gitextensions that referenced this pull request Mar 19, 2024
@pmiossec
fixup! FormResolveConflicts: Improve help when merging/rebasing (gite…
d2a8b7e 
…xtensions#11619)
@pmiossec pmiossec force-pushed the conflicts_little_help branch from a2af092 to d2a8b7e Compare March 19, 2024 15:06
@pmiossec
Copy link
Member Author

It will be most helpful to me to have these additions in the lower pane of the dialog (green arrows). This can be looked up without closing the merge tool.

Done.

@pmiossec
Copy link
Member Author

I would prefer to have the branch name where available, then commit, then remote/incoming (keep ours/theirs as it is used by Git).

I keep that in mind for later as it is a good idea but it's not easy to retrieve these information as we have no context when the form is opened. So I will definitively not do it in this PR which goal was a quick improvement on complaints I see here and there...

@gerhardol
Copy link
Member

I would prefer to have the branch name where available, then commit, then remote/incoming (keep ours/theirs as it is used by Git).

I keep that in mind for later as it is a good idea but it's not easy to retrieve these information as we have no context when the form is opened. So I will definitively not do it in this PR which goal was a quick improvement on complaints I see here and there...

I looked at this some years ago and saw no quick fix, I was hoping you had one...

I am OK with the change, even if I personally see no improvement.
The GE specific local/remote denominations could be replaced with incoming/current, that are better names (it is just having three names for the same thing that I do not like).

mstv
mstv reviewed Mar 19, 2024
View reviewed changes
GitUI/CommandsDialogs/FormResolveConflicts.Designer.cs Outdated
label7.TabIndex = 1;
label7.Text = "Local";
label7.Text = "Local/Current";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In dependency on the operation:

Suggested change
label7.Text = "Local/Current";
label7.Text = "Local/current (ours)";
Suggested change
label7.Text = "Local/Current";
label7.Text = "Local/current (theirs)";

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Required a refactoring (introducing class DisplayWithSuffixUpdater) to do the same thing we did to display keyboard shortcuts in tooltips.

Also:

  • added a tooltip:
    image

&

image

  • fixed documentation link that was broken
  • 2 other opportune refactorings

GitUI/CommandsDialogs/FormResolveConflicts.Designer.cs Outdated
label5.TabIndex = 5;
label5.Text = "Remote";
label5.Text = "Remote/Incoming";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Suggested change
label5.Text = "Remote/Incoming";
label5.Text = "Remote/incoming (...)";

pmiossec added a commit to pmiossec/gitextensions that referenced this pull request Mar 20, 2024
@pmiossec
fixup! FormResolveConflicts: Improve help when merging/rebasing (gite…
b379b17 
…xtensions#11619)
@pmiossec pmiossec force-pushed the conflicts_little_help branch from 9c6f422 to 8d9e4d8 Compare March 20, 2024 13:34
mstv
mstv reviewed Mar 20, 2024
View reviewed changes
Copy link
Member

@mstv mstv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, just nits

pmiossec added a commit to pmiossec/gitextensions that referenced this pull request Mar 20, 2024
@pmiossec
refactor: remove multiple calls to Module.InTheMiddleOfRebase() (gite…
04f86e6 
…xtensions#11619)
pmiossec added a commit to pmiossec/gitextensions that referenced this pull request Mar 20, 2024
@pmiossec
FormResolveConflicts: Improve help when merging/rebasing (gitextensio…
88150f0 
…ns#11619)

+ Add help tooltips on label
@pmiossec pmiossec force-pushed the conflicts_little_help branch from 8d9e4d8 to 88150f0 Compare March 20, 2024 21:04
pmiossec added a commit to pmiossec/gitextensions that referenced this pull request Mar 20, 2024
@pmiossec
FormResolveConflicts: Improve help/wording when merging/rebasing (git…
80b8b39 
…extensions#11619)

+ Add help tooltips
@pmiossec pmiossec force-pushed the conflicts_little_help branch from 88150f0 to 80b8b39 Compare March 20, 2024 21:07
mstv
mstv reviewed Mar 20, 2024
View reviewed changes
@pmiossec pmiossec force-pushed the conflicts_little_help branch from 80b8b39 to 4e07483 Compare March 20, 2024 22:06
@pmiossec pmiossec merged commit 3a7e5d8 into gitextensions:master Mar 21, 2024
pmiossec added a commit that referenced this pull request Mar 21, 2024
@pmiossec
FormResolveConflicts: fix help link (#11619)
27fb566 
following v4 doc update
pmiossec added a commit that referenced this pull request Mar 21, 2024
@pmiossec
refactor: clean InTheMiddleOf methods (#11619)
1347845 
* Prevent calculating rebase dir twice
* DRY
pmiossec added a commit that referenced this pull request Mar 21, 2024
@pmiossec
refactor: remove multiple calls to Module.InTheMiddleOfRebase() (#11619)
e2106bd
@pmiossec pmiossec deleted the conflicts_little_help branch March 21, 2024 08:48
@RussKie RussKie added this to the vNext milestone Mar 25, 2024
pmiossec added a commit to pmiossec/gitextensions that referenced this pull request Mar 28, 2024
@pmiossec
FormResolveConflicts: Avoid fragmented translation strings
9c707d5 
fixes gitextensions#11651

Introduced by 3a7e5d8 (gitextensions#11619)
@pmiossec pmiossec mentioned this pull request Mar 28, 2024
Merged
RussKie pushed a commit that referenced this pull request Mar 28, 2024
@pmiossec
FormResolveConflicts: Avoid fragmented translation strings (#11659)
52b29db 
fixes #11651

Introduced by 3a7e5d8 (#11619)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RussKie RussKie RussKie approved these changes

@mstv mstv mstv approved these changes

Assignees

@pmiossec pmiossec

Labels
None yet
Projects
None yet
Milestone
5.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@pmiossec @gerhardol @mstv @RussKie