Skip to content

Conversation

mstv
Copy link
Member

@mstv mstv commented Nov 24, 2024

Proposed changes

  • Remove mnemonic ampersand in script name except for tool bar and menus

Screenshots

image

Before After
image image
image image
image image

Test methodology

  • adapt 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 Nov 24, 2024
{
[GeneratedRegex("&(?!&)")]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
[GeneratedRegex("&(?!&)")]
// Match a single & (lookahead to not be followed by a second &)
[GeneratedRegex("&(?!&)")]

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you really still need such comments?

image

Copy link
Member

Choose a reason for hiding this comment

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

For lookahead it may be good to explain. Skip it if you prefer

Copy link
Member

Choose a reason for hiding this comment

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

I was going to ask for the comment as well...

@mstv mstv merged commit 97d0eae into gitextensions:master Nov 24, 2024
3 of 4 checks passed
@mstv mstv deleted the fix/script_name branch November 24, 2024 23:21
@mstv mstv added this to the v5.2 milestone Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants