mergetool feature request - select remote or local

mergetool feature request - select remote or local

[Caleb Cushing]

[-- Attachment #1: Type: text/plain, Size: 860 bytes --]

sometimes when merge-ing fast forward doesn't work. but you know what the 
resolution you want is.

example (current behavior)
Normal merge conflict for 'css/main.css':
  {local}: modified
  {remote}: modified
Hit return to start merge resolution tool (vimdiff):

but I don't want to. I know the remote updates are right. I could do a git 
checkout remotebranch filename but when you have 20 files that need updating 
this is annoying

my suggestion is this
Normal merge conflict for 'css/main.css':
  {local}: modified
  {remote}: modified
Use (l)local or (r)remote or (m)anual? 

also in the event of having 20 files with this issue it would be nice to have 
an option after first starting mergetool for remote all or local all.

(note: I am not subscribed to the list)
-- 
Caleb Cushing

my blog http://xenoterracide.blogspot.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: mergetool feature request - select remote or local

[Junio C Hamano]

Caleb Cushing <xenoterracide@gmail.com> writes:

> sometimes when merge-ing fast forward doesn't work. but you know what the 
> resolution you want is.
>
> example (current behavior)
> Normal merge conflict for 'css/main.css':
>   {local}: modified
>   {remote}: modified
> Hit return to start merge resolution tool (vimdiff):
>
> but I don't want to. I know the remote updates are right. I could do a git 
> checkout remotebranch filename but when you have 20 files that need updating 
> this is annoying

I never understood why people are using git and not ftp when they say the
other side is always right, but I see this comes up every once in a while,
so it would probably be a good thing to support.

> my suggestion is this
> Normal merge conflict for 'css/main.css':
>   {local}: modified
>   {remote}: modified
> Use (l)local or (r)remote or (m)anual? 

The above "(l)local" and "(m)anual" look inconsistent, and the wording
should be more like "local, remote or merge".

> also in the event of having 20 files with this issue it would be nice to have 
> an option after first starting mergetool for remote all or local all.

This makes me wonder if you are better off not using mergetool in such a
situation.  Instead, perhaps

	$ git ls-files -u --no-stage | xargs git checkout MERGE_HEAD

might be a better option?  The attached is a completely untested
weather-baloon patch.

If this turns out to be a better approach, perhaps we would further want
to tweak things to make:

	$ git checkout --unmerged MERGE_HEAD [--] [<pathspec>...]

to work (if you want "local", you would use "HEAD" instead of
"MERGE_HEAD").

I would have done so here if "git checkout" were still a scripted version,
but now it is in C, it would take significantly more effort than it is
worth just to raise a weatherbaloon.

 builtin-ls-files.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index dc7eab8..36b2875 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -181,6 +181,8 @@ static void show_ce_entry(const char *tag, struct cache_entry *ce)
 {
 	int len = prefix_len;
 	int offset = prefix_offset;
+	static const char *last_tag_shown;
+	static struct cache_entry *last_ce_shown;
 
 	if (len >= ce_namelen(ce))
 		die("git-ls-files: internal error - cache entry not superset of prefix");
@@ -206,7 +208,18 @@ static void show_ce_entry(const char *tag, struct cache_entry *ce)
 	}
 
 	if (!show_stage) {
+		/*
+		 * Even when not showing stage information, we will call this
+		 * function for each stage.  Omit duplicate output.
+		 */
+		if ((!tag ||
+		     (last_tag_shown && !strcmp(last_tag_shown, tag))) &&
+		    (last_ce_shown &&
+		     !strcmp(last_ce_shown->name, ce->name)))
+			return;
 		fputs(tag, stdout);
+		last_tag_shown = tag;
+		last_ce_shown = ce;
 	} else {
 		printf("%s%06o %s %d\t",
 		       tag,
@@ -509,6 +522,15 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 			show_unmerged = 1;
 			continue;
 		}
+		if (!strcmp(arg, "--no-stage")) {
+			/*
+			 * ... Not really.
+			 * Sometimes we would want a list of unmerged paths.
+			 */
+			show_stage = 0;
+			show_cached = 1;
+			continue;
+		}
 		if (!strcmp(arg, "-x") && i+1 < argc) {
 			exc_given = 1;
 			add_exclude(argv[++i], "", 0, &dir.exclude_list[EXC_CMDL]);

Re: mergetool feature request - select remote or local

[Daniel Barkalow]

On Wed, 14 May 2008, Junio C Hamano wrote:

> This makes me wonder if you are better off not using mergetool in such a
> situation.  Instead, perhaps
> 
> 	$ git ls-files -u --no-stage | xargs git checkout MERGE_HEAD
> 
> might be a better option?  The attached is a completely untested
> weather-baloon patch.
> 
> If this turns out to be a better approach, perhaps we would further want
> to tweak things to make:
> 
> 	$ git checkout --unmerged MERGE_HEAD [--] [<pathspec>...]
> 
> to work (if you want "local", you would use "HEAD" instead of
> "MERGE_HEAD").

This is "for all (or some, by pathspecs) files currently unmerged in the 
index, resolve them to the version in MERGE_HEAD", right?

> I would have done so here if "git checkout" were still a scripted version,
> but now it is in C, it would take significantly more effort than it is
> worth just to raise a weatherbaloon.

Like this (also untested)? (This is missing defaulting to a pathspec of 
'.' if --unmerged is used without any pathspecs)

diff --git a/builtin-checkout.c b/builtin-checkout.c
index 10ec137..0bae1d4 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -42,6 +42,8 @@ static int post_checkout_hook(struct commit *old, struct commit *new,
 	return run_command(&proc);
 }
 
+static int unmerged;
+
 static int update_some(const unsigned char *sha1, const char *base, int baselen,
 		       const char *pathname, unsigned mode, int stage)
 {
@@ -59,6 +61,13 @@ static int update_some(const unsigned char *sha1, const char *base, int baselen,
 	hashcpy(ce->sha1, sha1);
 	memcpy(ce->name, base, baselen);
 	memcpy(ce->name + baselen, pathname, len - baselen);
+	if (unmerged) {
+		int pos = cache_name_pos(ce->name, len);
+		if (!(pos && pos < active_nr && ce_same_name(active_cache[pos], active_cache[pos + 1]))) {
+			free(ce);
+			return 0;
+		}
+	}
 	ce->ce_flags = create_ce_flags(len, 0);
 	ce->ce_mode = create_ce_mode(mode);
 	add_cache_entry(ce, ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE);
@@ -508,6 +517,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 			BRANCH_TRACK_EXPLICIT),
 		OPT_BOOLEAN('f', NULL, &opts.force, "force"),
 		OPT_BOOLEAN('m', NULL, &opts.merge, "merge"),
+		OPT_BOOLEAN( 0 , "unmerged", &unmerged, "check out unmerged paths"),
 		OPT_END(),
 	};
 

Re: mergetool feature request - select remote or local

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> On Wed, 14 May 2008, Junio C Hamano wrote:
> ...
>> If this turns out to be a better approach, perhaps we would further want
>> to tweak things to make:
>> 
>> 	$ git checkout --unmerged MERGE_HEAD [--] [<pathspec>...]
>> 
>> to work (if you want "local", you would use "HEAD" instead of
>> "MERGE_HEAD").
>
> This is "for all (or some, by pathspecs) files currently unmerged in the 
> index, resolve them to the version in MERGE_HEAD", right?

Yeah, and possibly limited by pathspec as usual.

>> I would have done so here if "git checkout" were still a scripted version,
>> but now it is in C, it would take significantly more effort than it is
>> worth just to raise a weatherbaloon.
>
> Like this (also untested)? (This is missing defaulting to a pathspec of 
> '.' if --unmerged is used without any pathspecs)

Looks easy enough.  Caleb, does this approach fit the situation you
described better?

> diff --git a/builtin-checkout.c b/builtin-checkout.c
> index 10ec137..0bae1d4 100644
> --- a/builtin-checkout.c
> +++ b/builtin-checkout.c
> @@ -42,6 +42,8 @@ static int post_checkout_hook(struct commit *old, struct commit *new,
>  	return run_command(&proc);
>  }
>  
> +static int unmerged;
> +
>  static int update_some(const unsigned char *sha1, const char *base, int baselen,
>  		       const char *pathname, unsigned mode, int stage)
>  {
> @@ -59,6 +61,13 @@ static int update_some(const unsigned char *sha1, const char *base, int baselen,
>  	hashcpy(ce->sha1, sha1);
>  	memcpy(ce->name, base, baselen);
>  	memcpy(ce->name + baselen, pathname, len - baselen);
> +	if (unmerged) {
> +		int pos = cache_name_pos(ce->name, len);
> +		if (!(pos && pos < active_nr && ce_same_name(active_cache[pos], active_cache[pos + 1]))) {
> +			free(ce);
> +			return 0;
> +		}
> +	}
>  	ce->ce_flags = create_ce_flags(len, 0);
>  	ce->ce_mode = create_ce_mode(mode);
>  	add_cache_entry(ce, ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE);
> @@ -508,6 +517,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
>  			BRANCH_TRACK_EXPLICIT),
>  		OPT_BOOLEAN('f', NULL, &opts.force, "force"),
>  		OPT_BOOLEAN('m', NULL, &opts.merge, "merge"),
> +		OPT_BOOLEAN( 0 , "unmerged", &unmerged, "check out unmerged paths"),
>  		OPT_END(),
>  	};
>  

Re: mergetool feature request - select remote or local

[Caleb Cushing]

[-- Attachment #1: Type: text/plain, Size: 2234 bytes --]

On Wednesday 14 May 2008 01:47:09 pm you wrote:
> I never understood why people are using git and not ftp when they say the
> other side is always right, but I see this comes up every once in a while,
> so it would probably be a good thing to support.

well in my case I'm not particularly sure why fast forward didn't just merge 
them. Seems like I had resolved these changes in master previously. this 
problem is most annoying when I am resolving the same issues across multiple 
branches.

> The above "(l)local" and "(m)anual" look inconsistent, and the wording
> should be more like "local, remote or merge".

local,remote, merge is fine and I used the (m) for example because of how it 
asked me to handle a file existing locally (with mods?) but had been deleted 
in MERGE_HEAD?

> > also in the event of having 20 files with this issue it would be nice to
> > have an option after first starting mergetool for remote all or local
> > all.
>
> This makes me wonder if you are better off not using mergetool in such a
> situation.  Instead, perhaps
>
>         $ git ls-files -u --no-stage | xargs git checkout MERGE_HEAD

perhaps... I'm not this familiar with git yet.

> might be a better option?  The attached is a completely untested
> weather-baloon patch.
>
> If this turns out to be a better approach, perhaps we would further want
> to tweak things to make:
>
>         $ git checkout --unmerged MERGE_HEAD [--] [<pathspec>...]
>
> to work (if you want "local", you would use "HEAD" instead of
> "MERGE_HEAD").

> Looks easy enough.  Caleb, does this approach fit the situation you
> described better?

um.. is that just a resolution for an merge all (local or remote).

I'm gonna be honest when I say I'm an amateur developer (although I've 
considered myself a pretty good admin for a few years) and relatively new to 
git. So I'm getting a bit lost, but it sounds reasonable. It wouldn't affect 
any files that had been successfully fast forwarded right?

unfortunately I'm not sure how easy testing will be, as this problem doesn't 
arise with every merge, but has arisen often enough to be a pita.
-- 
Caleb Cushing

my blog http://xenoterracide.blogspot.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]