Skip to content

Conversation

lolgear
Copy link
Contributor

@lolgear lolgear commented Sep 7, 2019

@@ -916,6 +916,11 @@ - (IBAction)editSelectedCommitMessage:(id)sender {
[self editCommitMessage:commit];
}

- (IBAction)copySelectedCommitMesssage:(id)sender {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Messsage -> Message

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed :)

@@ -111,6 +111,12 @@ DQ
<action selector="editSelectedCommitMessage:" target="-2" id="PrK-v7-e6I"/>
</connections>
</menuItem>
<menuItem title="Copy Message…" alternate="YES" keyEquivalent="e" id="LnA-LI-dEb">
Copy link
Collaborator

Choose a reason for hiding this comment

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

... is reserved for actions where there will be something the user has to follow up on. There is no followup dialog here so the text should simply be Copy Message.

- (void)copyCommitMessage:(GCHistoryCommit*)commit {
NSString* message = _CleanedUpCommitMessage(commit.message);
[self.windowController showOverlayWithStyle:kGIOverlayStyle_Informational format:NSLocalizedString(@"Commit message copied: \"%@\"", nil), message];
[[NSPasteboard generalPasteboard] setString:message forType:NSPasteboardTypeString];
Copy link
Collaborator

@lucasderraugh lucasderraugh Sep 23, 2019

Choose a reason for hiding this comment

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

You don't have any -[NSPasteboard declareTypes:owner:] call. It's unlikely to be copying anything to the pasteboard in this case. From the docs: "The type must have been declared by a previous declareTypes:owner: message."

This goes along with my theme of don't make me test your code, please test to make sure it actually copies text...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lucasderraugh Heh, my bad. It is ridiculous how different macOS and iOS at some points. Yes, all applications in iOS are sandboxed, so, they can't harm user.

Mmm, I've found a bug in current release app. I think that "testing" code by others is one of the best practices to find bugs.

In this PR I was in hurry, yes. It is one of the core functions that I need ( besides drag & drop, history of file, search in commit view and search in selected range of commits ).

@lucasderraugh
Copy link
Collaborator

Very nice, looks great!

@lucasderraugh lucasderraugh merged commit 6a1a340 into git-up:master Sep 24, 2019
simpzan pushed a commit to simpzan/GitUp that referenced this pull request Oct 22, 2020
* kit: mapview copy commit message operation has been added.

* kit: mapview contextual menu entry copy selected commit message has been added.

* kit: copy selected commit message typo has been fixed.

* kit: views map view pasteboard declare types invocation has been added.

* kit: views map view contextual menu entry has been renamed.
@lolgear lolgear deleted the map_view_copy_commit_message_action branch May 17, 2021 09:33
@lolgear lolgear restored the map_view_copy_commit_message_action branch May 18, 2021 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants