Patchwork D1028: mpatch: reformat function prototypes with clang-format

login
register
mail settings
Submitter phabricator
Date Oct. 12, 2017, 3:09 p.m.
Message ID <differential-rev-PHID-DREV-q5ogcm73o6w7ovpy6ja5-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/24784/
State Superseded
Headers show

Comments

phabricator - Oct. 12, 2017, 3:09 p.m.
durin42 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1028

AFFECTED FILES
  mercurial/mpatch.c
  mercurial/mpatch.h

CHANGE DETAILS




To: durin42, #hg-reviewers
Cc: mercurial-devel
phabricator - Oct. 13, 2017, 1:19 p.m.
yuja requested changes to this revision.
yuja added a comment.
This revision now requires changes to proceed.


  Seemed fine, but test-check-code.t says no.
  
    +  mercurial/mpatch.c:268:
    +   >             ssize_t start, ssize_t end)
    +   don't use spaces to indent
    +  mercurial/mpatch.h:24:
    +   >             ssize_t start, ssize_t end);
    +   don't use spaces to indent

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1028

To: durin42, #hg-reviewers, pulkit, yuja
Cc: yuja, mercurial-devel
phabricator - Oct. 13, 2017, 5:40 p.m.
ryanmce added a comment.


  I thought it was string that check-code doesn't fire on https://phab.mercurial-scm.org/D1030 but does fire here.
  
  It turns out that the check-code only finds spaces at the start of the line. So is you start with a tab and then spaces, that's fine. But if you start with spaces you trip up against test-check-code. In https://phab.mercurial-scm.org/D1030, the re-indented line start with a tab, whereas here the re-indented line is top-level so it starts with spaces.
  
  I, for one, am -1 on tabs but that's not a battle worth fighting probably. However, we can probably come up with a smarter test-check-code. If we're moving towards clang-format, it seems that the test-code should be checking that and not arbitrarily for spaces at the start of a line.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1028

To: durin42, #hg-reviewers, pulkit, yuja
Cc: ryanmce, yuja, mercurial-devel
phabricator - Oct. 14, 2017, 7:07 a.m.
durin42 added a comment.


  In https://phab.mercurial-scm.org/D1028#17513, @yuja wrote:
  
  > Seemed fine, but test-check-code.t says no.
  >
  >   +  mercurial/mpatch.c:268:
  >   +   >             ssize_t start, ssize_t end)
  >   +   don't use spaces to indent
  >   +  mercurial/mpatch.h:24:
  >   +   >             ssize_t start, ssize_t end);
  >   +   don't use spaces to indent
  >
  
  
  
  
  In https://phab.mercurial-scm.org/D1028#17513, @yuja wrote:
  
  > Seemed fine, but test-check-code.t says no.
  >
  >   +  mercurial/mpatch.c:268:
  >   +   >             ssize_t start, ssize_t end)
  >   +   don't use spaces to indent
  >   +  mercurial/mpatch.h:24:
  >   +   >             ssize_t start, ssize_t end);
  >   +   don't use spaces to indent
  >
  
  
  Ah. Okay. Since everyone at the sprint seemed loosely in favor of clang-format, how about this: I'll get the remaining trivial fixes from clang-format in, then land a test-check-clang-format.t with an extensive blacklist, then remove the check-code rules that are offended by clang-format as the series exposes them. That way we own less checking, and the formatting is done by a robot.
  
  That work?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1028

To: durin42, #hg-reviewers, pulkit, yuja
Cc: ryanmce, yuja, mercurial-devel
phabricator - Oct. 14, 2017, 7:09 a.m.
durin42 added a comment.


  In https://phab.mercurial-scm.org/D1028#17622, @ryanmce wrote:
  
  > I, for one, am -1 on tabs but that's not a battle worth fighting probably. However, we can probably come up with a smarter test-check-code. If we're moving towards clang-format, it seems that the test-code should be checking that and not arbitrarily for spaces at the start of a line.
  
  
  I'm going to forge ahead with the coding style we nominally have in the codebase today, and we can consider changes once we're done with the initial move to the formatter. Sound good?
  
  (I don't feel especially strongly, though my personal preference is still tabs-for-indentation spaces-for-alignment, which clang-format happily can do. I just have better things to argue about, which is why clang-format is great, especially if you get editor integration set up.)

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1028

To: durin42, #hg-reviewers, pulkit, yuja
Cc: ryanmce, yuja, mercurial-devel

Patch

diff --git a/mercurial/mpatch.h b/mercurial/mpatch.h
--- a/mercurial/mpatch.h
+++ b/mercurial/mpatch.h
@@ -14,13 +14,13 @@ 
 	struct mpatch_frag *base, *head, *tail;
 };
 
-int mpatch_decode(const char *bin, ssize_t len, struct mpatch_flist** res);
+int mpatch_decode(const char *bin, ssize_t len, struct mpatch_flist **res);
 ssize_t mpatch_calcsize(ssize_t len, struct mpatch_flist *l);
 void mpatch_lfree(struct mpatch_flist *a);
 int mpatch_apply(char *buf, const char *orig, ssize_t len,
-	struct mpatch_flist *l);
-struct mpatch_flist *mpatch_fold(void *bins,
-	struct mpatch_flist* (*get_next_item)(void*, ssize_t),
-	ssize_t start, ssize_t end);
+                 struct mpatch_flist *l);
+struct mpatch_flist *
+mpatch_fold(void *bins, struct mpatch_flist *(*get_next_item)(void *, ssize_t),
+            ssize_t start, ssize_t end);
 
 #endif
diff --git a/mercurial/mpatch.c b/mercurial/mpatch.c
--- a/mercurial/mpatch.c
+++ b/mercurial/mpatch.c
@@ -64,7 +64,7 @@ 
    for changes in offset. the last hunk may be split if necessary.
 */
 static int gather(struct mpatch_flist *dest, struct mpatch_flist *src, int cut,
-	int offset)
+                  int offset)
 {
 	struct mpatch_frag *d = dest->tail, *s = src->head;
 	int postend, c, l;
@@ -147,7 +147,7 @@ 
 /* combine hunk lists a and b, while adjusting b for offset changes in a/
    this deletes a and b and returns the resultant list. */
 static struct mpatch_flist *combine(struct mpatch_flist *a,
-	struct mpatch_flist *b)
+                                    struct mpatch_flist *b)
 {
 	struct mpatch_flist *c = NULL;
 	struct mpatch_frag *bh, *ct;
@@ -241,7 +241,7 @@ 
 }
 
 int mpatch_apply(char *buf, const char *orig, ssize_t len,
-	struct mpatch_flist *l)
+                 struct mpatch_flist *l)
 {
 	struct mpatch_frag *f = l->head;
 	int last = 0;
@@ -263,9 +263,9 @@ 
 }
 
 /* recursively generate a patch of all bins between start and end */
-struct mpatch_flist *mpatch_fold(void *bins,
-	struct mpatch_flist* (*get_next_item)(void*, ssize_t),
-	ssize_t start, ssize_t end)
+struct mpatch_flist *
+mpatch_fold(void *bins, struct mpatch_flist *(*get_next_item)(void *, ssize_t),
+            ssize_t start, ssize_t end)
 {
 	ssize_t len;