Add DiffMerge tool support #481
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@akantsevoi I've recently taken over as temporary maintainer of GitUp. If I can get an overview of visual changes, I'd love to start the discussion on merging this in. |
|
Hi @lucasderraugh , nice to meet you. What do you mean about visual changes? Some screenshots or like that? |
|
@akantsevoi Nice to meet you too! Screenshots of any visual changes would be great! Along with just a brief explanation of the code and what you're trying to accomplish with it. Just helps when I'm testing out the change to know what differences I should be looking for. |
| [arguments addObject: ourPath]; | ||
| [arguments addObject: ancestorPath]; | ||
| [arguments addObject: theirPath]; | ||
| [self _runDiffMergeToolWithArguments: arguments]; |
There was a problem hiding this comment.
Could we remove the spacing after the colon in these 4 spots. Other than that, code wise this looks good to me.
|
You can just run format-source script after merging
…On Sun, Jun 9, 2019 at 1:37 PM Lucas Derraugh ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In GitUpKit/Utilities/GIViewController+Utilities.m
<#481 (comment)>:
> @@ -571,6 +583,15 @@ - (void)resolveConflictInMergeTool:(GCIndexConflict*)conflict {
[self _runP4MergeWithArguments:arguments];
} else if ([identifier isEqualToString:GIViewControllerTool_GitTool]) {
[self _runMergeGitToolForFile:mergePath withOldPath:ourPath newPath:theirPath basePath:ancestorPath];
+ } else if ([identifier isEqualToString:GIViewControllerTool_DiffMerge]) {
+ [arguments addObject:[NSString stringWithFormat:@"-r=%@", mergePath]];
+ [arguments addObject:[NSString stringWithFormat:@"-t1=%@", ourTitle]];
+ [arguments addObject:[NSString stringWithFormat:@"-t2=%@", ancestorTitle]];
+ [arguments addObject:[NSString stringWithFormat:@"-t3=%@", theirTitle]];
+ [arguments addObject: ourPath];
+ [arguments addObject: ancestorPath];
+ [arguments addObject: theirPath];
+ [self _runDiffMergeToolWithArguments: arguments];
Could we remove the spacing after the colon in these 4 spots. Other than
that, code wise this looks good to me.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#481?email_source=notifications&email_token=AAEZJS3V5JGPNWHFXYMKKMLPZVSWTA5CNFSM4F2YFCU2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB27TZGY#pullrequestreview-247413915>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAEZJSZI47X42XCP2A4S5NDPZVSWTANCNFSM4F2YFCUQ>
.
|
|
@swisspol Do you just run that periodically and make a new commit? Would be nice if that could somehow be done before merging, but it's good to know that script exists. |
|
I just ran it by hand before committing or sometimes after PRs
…On Sun, Jun 9, 2019 at 2:14 PM Lucas Derraugh ***@***.***> wrote:
@swisspol <https://github.com/swisspol> Do you just run that periodically
and make a new commit? Would be nice if that could somehow be done before
merging, but it's good to know that script exists.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#481?email_source=notifications&email_token=AAEZJS5PCSPDZJMYPXWGQN3PZVXDVA5CNFSM4F2YFCU2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXISMHI#issuecomment-500246045>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAEZJS45TGO665MEU3RPI6LPZVXDVANCNFSM4F2YFCUQ>
.
|
Updated to clang-format 8.0.0
|
@akantsevoi How would I go about testing this change? Some steps to verify would be great so I can push this change along. Thanks! |
…ing (#528) This addresses a crash that became consistent on macOS Catalina, but was ultimately caused by the early freeing of a git_repository instance, by closing and reopening a repository whenever its location has changed on the filesystem. In order to support this change, window frame saving is suspended for the reopened repository in order to avoid writing to the old location of the repository. Finally, the welcome window is no longer shown when all documents close to prevent it from appearing during one of these re-openings. Clicking on the GitUp icon in the dock will still show the welcome window. This behaves similarly to Xcode.
* Perform Xcode 10.2 modernization * Update deployment target to 10.9 * Fixes libgit2 reference in GitUpKit for iOS * Make all targets use shared config * Adopt base localization * Update the tools used to build on CI * Adopt guarded availability checking * NSViewController is part of the responder chain after 10.10 * NSViewController has native appearance callbacks after 10.10 * Fix trivial warnings * Update deployment target to 10.10
* Add a helper for looking up the GitUpKit bundle * Add polyfills for NSAppearance * Enable dark mode * Draw consistent grid for diffs with line number gutters in dark mode * Remove map status label backgrounds on Mojave For some reason the label backgrounds did not match the window background in dark mode. As I understand it, Mojave no longer uses sub-pixel antialiasing so this workaround is no longer needed. * Add a namespace for defining semantic colors * Allow window-tinted background in graph view * Support Dark Mode in modal views * Fix dark-on-dark text in selected cells in dark mode * Don't fill the background color for conflict/empty rows in diff views * Support Dark Mode in welcome window * Support Dark Mode in main window toolbar * Support Dark Mode in main window status bar * Support Dark Mode in window tip overlays * Support Dark Mode in map view * Support Dark Mode in diff views * Support Dark Mode in commit editors * Support Dark Mode in commit viewers * Support Dark Mode in commit lists * Improve branch colors More closely based on the system colors like systemRedColor. Adds orange instead of using two variants of green. Uses more saturated colors in Dark Mode * Support Dark Mode in config editor * Update Document.m Fix bad merge from master
* Remove extra indirection in file lists * Use native table view drag handling in diff views * Use the common dragging implementation for copying in a diff files view * Provide the plain-text relative path for diff view drag and drop * Allow dragging files out of the working copy into external apps * Allow dragging files out of diffs into external apps
|
Somehow there is a merge commit that I didn't want but I guess GitHub Desktop insisted on and so it's screwed this branch. Definitely don't want to make the changes that I've put on here now, but I'll take what you had and create a new PR. Sorry for this, was just trying to resolve a xib file conflict... |
lucasderraugh
added a commit
that referenced
this pull request
Jul 15, 2019
Changes come from original PR #481 made by @akantsevoi. Fixes #59.
|
Sorry @akantsevoi. I've rebased your changes on master and they're set to be taken whenever I can fix the continuous build signing issues. Your changes were added with 6aa8df8 |
simpzan
pushed a commit
to simpzan/GitUp
that referenced
this pull request
Oct 22, 2020
Changes come from original PR git-up#481 made by @akantsevoi. Fixes git-up#59.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Close the issue #59