Re: [PATCH v3 00/11] rerere: handle nested conflicts

2018-07-30 Thread Thomas Gummerer
On 07/30, Junio C Hamano wrote:
> Thomas Gummerer  writes:
> 
> > Thomas Gummerer (11):
> >   rerere: unify error messages when read_cache fails
> >   rerere: lowercase error messages
> >   rerere: wrap paths in output in sq
> >   rerere: mark strings for translation
> >   rerere: add documentation for conflict normalization
> >   rerere: fix crash when conflict goes unresolved
> >   rerere: only return whether a path has conflicts or not
> >   rerere: factor out handle_conflict function
> >   rerere: return strbuf from handle path
> >   rerere: teach rerere to handle nested conflicts
> >   rerere: recalculate conflict ID when unresolved conflict is committed
> 
> Even though I am not certain about the last two steps, everything
> before them looked trivially correct and good changes (well, the
> "strbuf" one's goodness obviously depends on the goodness of the
> last two, which are helped by it).
> 
> Sorry for taking so long before getting to the series.

No worries, I realize you are busy with a lot of other things.  Thanks
a lot for your review!


Re: [PATCH v3 00/11] rerere: handle nested conflicts

2018-07-30 Thread Junio C Hamano
Thomas Gummerer  writes:

> Thomas Gummerer (11):
>   rerere: unify error messages when read_cache fails
>   rerere: lowercase error messages
>   rerere: wrap paths in output in sq
>   rerere: mark strings for translation
>   rerere: add documentation for conflict normalization
>   rerere: fix crash when conflict goes unresolved
>   rerere: only return whether a path has conflicts or not
>   rerere: factor out handle_conflict function
>   rerere: return strbuf from handle path
>   rerere: teach rerere to handle nested conflicts
>   rerere: recalculate conflict ID when unresolved conflict is committed

Even though I am not certain about the last two steps, everything
before them looked trivially correct and good changes (well, the
"strbuf" one's goodness obviously depends on the goodness of the
last two, which are helped by it).

Sorry for taking so long before getting to the series.


[PATCH v3 00/11] rerere: handle nested conflicts

2018-07-14 Thread Thomas Gummerer
The previous rounds were at
<20180520211210.1248-1-t.gumme...@gmail.com> and
<20180605215219.28783-1-t.gumme...@gmail.com>.

This round is a more polished version of the previous round, as
suggested by Junio in .
It's also rebased on v2.18.

The series grew by one patch, because 8/10 has been split into two
patches hopefully making it easier to follow.

range-diff before below:

1:  2825342cc2 = 1:  018bd68a8a rerere: unify error messages when read_cache 
fails
2:  d1500028aa ! 2:  281fcbf24f rerere: lowercase error messages
@@ -4,7 +4,8 @@
 
 Documentation/CodingGuidelines mentions that error messages should be
 lowercase.  Prior to marking them for translation follow that pattern
-in rerere as well.
+in rerere as well, so translators won't have to translate messages
+that don't conform to our guidelines.
 
 Signed-off-by: Thomas Gummerer 
 
@@ -87,3 +88,12 @@
  
/* Nuke the recorded resolution for the conflict */
id = new_rerere_id(sha1);
+@@
+   handle_cache(path, sha1, rerere_path(id, "thisimage"));
+   if (read_mmfile(, rerere_path(id, "thisimage"))) {
+   free(cur.ptr);
+-  error("Failed to update conflicted state in '%s'", 
path);
++  error("failed to update conflicted state in '%s'", 
path);
+   goto fail_exit;
+   }
+   cleanly_resolved = !try_merge(id, path, , );
3:  ed3601ee71 = 3:  b6d5e2e26d rerere: wrap paths in output in sq
4:  6ead84a199 ! 4:  45f0d7a99f rerere: mark strings for translation
@@ -4,7 +4,7 @@
 
 'git rerere' is considered a plumbing command and as such its output
 should be translated.  Its functionality is also only enabled through
-a config setting, so scripts really shouldn't rely on its output
+a config setting, so scripts really shouldn't rely on the output
 either way.
 
 Signed-off-by: Thomas Gummerer 
@@ -219,8 +219,8 @@
handle_cache(path, sha1, rerere_path(id, "thisimage"));
if (read_mmfile(, rerere_path(id, "thisimage"))) {
free(cur.ptr);
--  error("Failed to update conflicted state in '%s'", 
path);
-+  error(_("Failed to update conflicted state in '%s'"), 
path);
+-  error("failed to update conflicted state in '%s'", 
path);
++  error(_("failed to update conflicted state in '%s'"), 
path);
goto fail_exit;
}
cleanly_resolved = !try_merge(id, path, , );
5:  caad276aca ! 5:  993857a816 rerere: add some documentation
@@ -1,6 +1,6 @@
 Author: Thomas Gummerer 
 
-rerere: add some documentation
+rerere: add documentation for conflict normalization
 
 Add some documentation for the logic behind the conflict normalization
 in rerere.
@@ -27,30 +27,28 @@
 +when different conflict style settings are used, rerere normalizes the
 +conflicts before writing them to the rerere database.
 +
-+Differnt conflict styles and branch names are dealt with by stripping
-+that information from the conflict markers, and removing extraneous
-+information from the `diff3` conflict style.
-+
-+Branches being merged in different order are dealt with by sorting the
-+conflict hunks.  More on each of those parts in the following
-+sections.
++Different conflict styles and branch names are normalized by stripping
++the labels from the conflict markers, and removing extraneous
++information from the `diff3` conflict style. Branches that are merged
++in different order are normalized by sorting the conflict hunks.  More
++on each of those steps in the following sections.
 +
 +Once these two normalization operations are applied, a conflict ID is
-+created based on the normalized conflict, which is later used by
++calculated based on the normalized conflict, which is later used by
 +rerere to look up the conflict in the rerere database.
 +
 +Stripping extraneous information
 +
 +
 +Say we have three branches AB, AC and AC2.  The common ancestor of
-+these branches has a file with with a line with the string "A" (for
-+brevity this line is called "line A" for brevity in the following) in
-+it.  In branch AB this line is changed to "B", in AC, this line is
-+changed to C, and branch AC2 is forked off of AC, after the line was
-+changed to C.
++these branches has a file with a line containing the string "A" (for
++brevity this is called "line A" in the rest of the document).  In
++branch AB this line is changed to "B", in AC, this line is changed to
++"C", and branch AC2 is forked off of AC, after the line was changed to