timbrel / GitSavvy

Full git and GitHub integration with Sublime Text
MIT License
1.91k stars 137 forks source link

Feature request: attempt to execute mergetool with common tools #266

Open asfaltboy opened 9 years ago

asfaltboy commented 9 years ago

When git does not find a merge.tool config entry, it attempts to run the first "mergetool" it finds, from a list of pre-defined tools:

This message is displayed because 'merge.tool' is not configured. See 'git mergetool --tool-help' or 'git help config' for more details. 'git mergetool' will now attempt to use one of the following tools: opendiff kdiff3 tkdiff xxdiff meld tortoisemerge gvimdiff diffuse diffmerge ecmerge p4merge araxis bc codecompare vimdiff emerge Merging: Default.sublime-commands

Normal merge conflict for 'Default.sublime-commands': {local}: modified file {remote}: modified file Hit return to start merge resolution tool (opendiff):

As seen above, the tools it attempts (ordered, I believe, from left to right) are:

opendiff kdiff3 tkdiff xxdiff meld tortoisemerge gvimdiff diffuse diffmerge ecmerge p4merge araxis bc codecompare vimdiff emerge

It would be helpful if we attempted the same, possibly prompting the user prior to running the located tool (as git does).

stoivo commented 9 years ago

I personally never uses a merge tool.

How would you like it to work than? I have one idea.

  1. User ask to open Merge tool ( merge tool is not set)
  2. We check what merge tools is available on system
  3. Ask use to select one of them (maybe set it as default merge tool in git config or set it as a setting in sublime and autoselect it next time, so the user only have to press enter)
  4. Open merge tool
asfaltboy commented 9 years ago

Sounds good to me! Some notes:

(2) For merge tools available, we can do the same as git, using common tools found in most systems and branching out on is_gnome condition.

(3) :+1: to saving the user selection in sublime GitSavvy settings (need to document this in case someone wants to go back and change this manually).

(4) Opening a tool is more complex than it sounds (each tool has it's arguments/usage), so we should probably not reinvent the wheel, and use again use git. I think it's simplest is to relaunch the git mergetool, providing the -t/--tool command and passing in the selected tool name.

stoivo commented 9 years ago

yeah, can you explain 2. step a bit more. Maybe I don't understand the shell script correctly

asfaltboy commented 9 years ago

No you're right, the script jumps around a bit, in the end it uses the first tool that it finds. To find that, it uses "type $tool" and tests if the tool exist:

$ type sdfadfg
-bash: type: sdfadfg: not found
$ echo $?
1

$ type git
git is aliased to `hub'
$ echo $?
0

$ type hub
hub is /usr/local/bin/hub
$ echo $?
0

This test seems to be the go to way to testing for file existence in git source.

NOTE: An issue with this method is that "type" does something else on windows and git for windows, works around it by using a shell wrapper. Moreover, it seems possible to run "type" from git windows wrapper shell (mingw), and test that result:

C:\>"%SYSTEMDRIVE%\Program Files (x86)\Git\bin\sh.exe" -c "type xxxx;echo $?"
/usr/bin/bash: line 0: type: xxxx: not found
1

C:\>"%SYSTEMDRIVE%\Program Files (x86)\Git\bin\sh.exe" -c "type git;echo $?"
git is /mingw32/bin/git
0
divmain commented 9 years ago

I think this is going to be tricky, and potentially bug-prone for Windows especially. GitSavvy has no special knowledge of a Cygwin environment (or Git's happy mingw world).

I hesitate to add support for this when the work-around is so explicit and straightforward: add a merge tool entry to your Git config. On slower systems, there could be a long-ish delay as GitSavvy checks the sequence of possible merge tools. We could introduce a GitSavvy option so that it only does these checks if use explicitly says so. But this seems inferior to the user just setting their merge tool config.

asfaltboy commented 9 years ago

I have to disagree here. For me, the whole point of using GitSavvy is to avoid remembering / understanding git commands and configuration keys; I expect it to do it for me. Moreover, in this case, git's own implementation is superior in it's simplicity. Let me explain.

First, when merge.tool is configured, git uses the tool's executable found on PATH, while GitSavvy attempts to use mergetool.<tool>.cmd and will fail, since it is not required to be set.

Second, the delay that may be experienced on a slow system would also occur just the same if the user runs git mergetool from his shell; if we indeed implement this the same as git does.

We don't even have to maintain a list of tools per system, since git has a handy git mergetool --tool-help option which prints out a handy guide on which tools are available to be specified in git mergetool --tool=:

'git mergetool --tool=<tool>' may be set to one of the following:
        emerge
        gvimdiff
        gvimdiff2
        gvimdiff3
        opendiff
        vimdiff
        vimdiff2
        vimdiff3

    user-defined:
        diffmerge
        sourcetree

The following tools are valid, but not currently available:
        araxis
        bc
        bc3
        codecompare
        deltawalker
        diffmerge
        diffuse
        ecmerge
        kdiff3
        meld
        p4merge
        tkdiff
        tortoisemerge
        xxdiff

Some of the tools listed above only work in a windowed
environment. If run in a terminal-only session, they will fail.

As for windows goes, yes it is usually difficult to test / maintain windows but, the way I see it, what git does so can we, with the help of some of GitSavvy's windows users.

divmain commented 9 years ago

That's a strong argument. What do you think about this @asfaltboy?

  1. run git mergetool --tool-help and parse the output
  2. search the PATH for any that are available
  3. if any are found, ask the user if they'd to use one on an ongoing basis
  4. if they say "Yes", display all options in a quick panel, and do the git config for them

The only potentially weak point in there is parsing the output of git mergetool --tool-help - we'd have to check whether the format is consistent across Git versions.

asfaltboy commented 9 years ago

Yep, that's about what I thought, only I feel the selected tool should be a sublime setting, rather then git config.

Also, I'm looking at MergeMixin, and perhaps we can simplify the logic by passing tool choice to git mergetool --tool=<choice>, rather than constructing our own command. The choice in this case is the tool name, rather than path, so we don't even have to care about looking up in PATH.

This call can be spawned from either a thread (as in custom command) or from a subprocess, like now.

EDIT: A quick-and-dirty implementation @ asfaltboy/GitSavvy@b69cac6

asfaltboy commented 9 years ago

The only issue I've bumped into so far is that some tools (for me, emerge and diffmerge) must be run in a terminal, which does not automatically get launched (duh), so a bit of unfriendly behaviour there. But I think it's same behavior right now.

Only thing that's left is to decide whether to store the selected too and where to store it (git conf or sublime).

asfaltboy commented 8 years ago

The mentioned code prompts for selection of tool and then attempts to simplify the tool launch by using git's mergetool.sh; however, that helper can potentially prompt the user, even when given an explicit tool, namely for:

If we want to go this route, we would also need to handle these scenarios (which may be a nice feature of itself), prompt user for a decision (delete / keep that or that) and complete action.

Otherwise, we would have to do some kind of setup for a tool. git does it by sourcing a tool's shell script which contains redefined merge_cmd function.

However, determining a merge_cmd for (non user-configured) tools may prove to be difficult to mimic.

I believe it's in our best interest to use mergetool as is for "simple" conflicting files, and add at least minimum behavior to cover other conflicts mentioned above. This also has the benifit of completing the missing functionality in regards with conflict handling for deleted/symlinked/submodule/failed files; and in most cases follows this flow:

  1. Detect conflict type
  2. Prompt user for a selection
  3. Perform git checkout/add/rm.
  4. Profit!