Skip to content
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

Fix: "Ctrl+Enter" creates commit when Enter is released, not pressed #10742

Merged
merged 6 commits into from Feb 26, 2023
Merged

Fix: "Ctrl+Enter" creates commit when Enter is released, not pressed #10742

merged 6 commits into from Feb 26, 2023

Conversation

RauulDev
Copy link
Contributor

@RauulDev RauulDev commented Feb 24, 2023 •
edited

Proposed changes

  • Fix: Replaced KeyUp event with KeyDown in order to handle commit action in FormCommit.cs

Test methodology

  • Manual and I ran all tests:
    image

Test environment(s)

  • GIT git version 2.33.0.windows.2
  • Windows 10 Pro 21H2 19044.2486

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.

@ghost ghost assigned RauulDev Feb 24, 2023
rmolina added 2 commits February 24, 2023 13:44
Revert "Fix KeyUp event to KeyDown in order to handle commit action"
5670c40 
This reverts commit b8d175c.

Revert "Removed KeyUp event from Message control."

This reverts commit 99f7a47.
Removed KeyUp event from Message control.
3476763
@RauulDev RauulDev changed the title Bugfix 7791 Fix: "Ctrl+Enter" creates commit when Enter is released, not pressed Feb 24, 2023
mstv
mstv approved these changes Feb 24, 2023
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.

LGTM, works well

GitUI/CommandsDialogs/FormCommit.cs Outdated
Comment on lines 2814 to 2815
// Prevent adding a line break when all we want is to commit
// Ctrl + Enter = Commit
Copy link
Member

Choose a reason for hiding this comment

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

I would rather remove both comments.

Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@RussKie RussKie added 🖊️ status: cla signed 📭 needs: author feedback More info/confirmation awaited from OP; issues typically get closed after 30 days of inactivity labels Feb 25, 2023
gerhardol
gerhardol reviewed Feb 25, 2023
View reviewed changes
Copy link
Member

@gerhardol gerhardol left a comment

Choose a reason for hiding this comment

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

+1

@ghost ghost removed the 📭 needs: author feedback More info/confirmation awaited from OP; issues typically get closed after 30 days of inactivity label Feb 26, 2023
@mstv mstv merged commit 04dafda into gitextensions:master Feb 26, 2023
@ghost ghost added this to the vNext milestone Feb 26, 2023
@mstv
Copy link
Member

mstv commented Feb 26, 2023

Thank you

@RauulDev RauulDev deleted the bugfix_7791 branch February 26, 2023 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gerhardol gerhardol gerhardol left review comments

@RussKie RussKie RussKie approved these changes

@mstv mstv mstv approved these changes

Assignees

@RauulDev RauulDev

Labels
🖊️ status: cla signed
Projects
None yet
Milestone
4.1
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@RauulDev @mstv @RussKie @gerhardol