Patchwork formatting: apply clang-format using the format-source extension

login
register
mail settings
Submitter Boris Feld
Date Jan. 14, 2019, 10:49 a.m.
Message ID <1b6f196b7e094278f995.1547462964@localhost.localdomain>
Download mbox | patch
Permalink /patch/37720/
State New
Headers show

Comments

Boris Feld - Jan. 14, 2019, 10:49 a.m.
# HG changeset patch
# User Boris Feld <boris.feld@octobus.net>
# Date 1547462137 -3600
#      Mon Jan 14 11:35:37 2019 +0100
# Node ID 1b6f196b7e094278f9956a479e10e02cd1108a12
# Parent  8d0d695fc7910665452b81052d1bbc55b3829b14
# EXP-Topic format-source
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 1b6f196b7e09
formatting: apply clang-format using the format-source extension

This applies formatting on the source code, similar to what `make format-c`
does.

(This catch a lack of formatting in one of our file).

Using the format source extension means this transformation is tracked in the
history and can be reused during merges. This smooth the consequence of
formatting changes between branches (and for in-progress works).


In theory, the initial formatting is done through format-source. However,
doing it after the fact is still beneficial as it will track the configuration
changes.

We have two main motivations to do this. First, we want to have format-source
used and testing in a wider range of real-world use case. This will help the
extension to mature before we submit it for upstream inclusion. Second, we got
impacted by formatting change during the 4.9 cycle, having format-source
enabled will help us in the future.

This formatting has been performed with format-source version 3.3.

We also plan to use format-source for rust formatting (and python formatting
once a tool has been picked).
Augie Fackler - Jan. 14, 2019, 2:57 p.m.
> On Jan 14, 2019, at 05:49, Boris Feld <boris.feld@octobus.net> wrote:
> 
> # HG changeset patch
> # User Boris Feld <boris.feld@octobus.net>
> # Date 1547462137 -3600
> #      Mon Jan 14 11:35:37 2019 +0100
> # Node ID 1b6f196b7e094278f9956a479e10e02cd1108a12
> # Parent  8d0d695fc7910665452b81052d1bbc55b3829b14
> # EXP-Topic format-source
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 1b6f196b7e09
> formatting: apply clang-format using the format-source extension
> 
> This applies formatting on the source code, similar to what `make format-c`
> does.
> 
> (This catch a lack of formatting in one of our file).
> 
> Using the format source extension means this transformation is tracked in the
> history and can be reused during merges. This smooth the consequence of
> formatting changes between branches (and for in-progress works).
> 
> 
> In theory, the initial formatting is done through format-source. However,
> doing it after the fact is still beneficial as it will track the configuration
> changes.
> 
> We have two main motivations to do this. First, we want to have format-source
> used and testing in a wider range of real-world use case. This will help the
> extension to mature before we submit it for upstream inclusion.

Do you have a plan for how to integrate format-source with fix?

Also, how does `format-source` cope with different versions of clang-format being installed? Our tests, as an example, have to check the major version of the tool before applying it, because older versions do some things differently. When I asked the clang-format devs about this their only advice was to either do version checks or check in the clang-format binary your developers should use...


> Second, we got
> impacted by formatting change during the 4.9 cycle, having format-source
> enabled will help us in the future.
> 
> This formatting has been performed with format-source version 3.3.
> 
> We also plan to use format-source for rust formatting (and python formatting
> once a tool has been picked).
> 
> diff --git a/.hg-format-source b/.hg-format-source
> new file mode 100644
> --- /dev/null
> +++ b/.hg-format-source
> @@ -0,0 +1,1 @@
> +{"configpaths": [".clang-format", "contrib/clang-format-ignorelist"], "pattern": "set:(**.c or **.cc or **.h) and not \"listfile:contrib/clang-format-ignorelist\"", "tool": "clang-format"}
> diff --git a/mercurial/cext/revlog.c b/mercurial/cext/revlog.c
> --- a/mercurial/cext/revlog.c
> +++ b/mercurial/cext/revlog.c
> @@ -995,8 +995,8 @@ static inline int index_baserev(indexObj
> 	if (result < -1) {
> 		PyErr_Format(
> 		    PyExc_ValueError,
> -		    "corrupted revlog, revision base out of range: %d, %d",
> -		    rev, result);
> +		    "corrupted revlog, revision base out of range: %d, %d", rev,
> +		    result);
> 		return -2;
> 	}
> 	return result;
Boris Feld - Jan. 16, 2019, 7:28 a.m.
On 14/01/2019 15:57, Augie Fackler wrote:
>
>> On Jan 14, 2019, at 05:49, Boris Feld <boris.feld@octobus.net> wrote:
>>
>> # HG changeset patch
>> # User Boris Feld <boris.feld@octobus.net>
>> # Date 1547462137 -3600
>> #      Mon Jan 14 11:35:37 2019 +0100
>> # Node ID 1b6f196b7e094278f9956a479e10e02cd1108a12
>> # Parent  8d0d695fc7910665452b81052d1bbc55b3829b14
>> # EXP-Topic format-source
>> # Available At https://bitbucket.org/octobus/mercurial-devel/
>> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 1b6f196b7e09
>> formatting: apply clang-format using the format-source extension
>>
>> This applies formatting on the source code, similar to what `make format-c`
>> does.
>>
>> (This catch a lack of formatting in one of our file).
>>
>> Using the format source extension means this transformation is tracked in the
>> history and can be reused during merges. This smooth the consequence of
>> formatting changes between branches (and for in-progress works).
>>
>>
>> In theory, the initial formatting is done through format-source. However,
>> doing it after the fact is still beneficial as it will track the configuration
>> changes.
>>
>> We have two main motivations to do this. First, we want to have format-source
>> used and testing in a wider range of real-world use case. This will help the
>> extension to mature before we submit it for upstream inclusion.
> Do you have a plan for how to integrate format-source with fix?
If i remember correctly, Martijn and Daniel discussed some of that at
the sprint.
>
> Also, how does `format-source` cope with different versions of clang-format being installed? Our tests, as an example, have to check the major version of the tool before applying it, because older versions do some things differently. When I asked the clang-format devs about this their only advice was to either do version checks or check in the clang-format binary your developers should use...

TL;DR; we know of the issue, actual solution to be decided

That question is on the stack of things we would like to deal with
during the extensions maturation. A strict way to do this would be to
use different "tool" id for each version, but that is more cumbersome
than we would like. If we could request strict versioned behavior from
tools that would be ideal, but this is probably too optimistic. So we
probably need an in between, tracking tool version and detecting changes.

We started discussing it with the black author who got intrigued at the
use case. More discussion planned with him soon.
>
>
>> Second, we got
>> impacted by formatting change during the 4.9 cycle, having format-source
>> enabled will help us in the future.
>>
>> This formatting has been performed with format-source version 3.3.
>>
>> We also plan to use format-source for rust formatting (and python formatting
>> once a tool has been picked).
>>
>> diff --git a/.hg-format-source b/.hg-format-source
>> new file mode 100644
>> --- /dev/null
>> +++ b/.hg-format-source
>> @@ -0,0 +1,1 @@
>> +{"configpaths": [".clang-format", "contrib/clang-format-ignorelist"], "pattern": "set:(**.c or **.cc or **.h) and not \"listfile:contrib/clang-format-ignorelist\"", "tool": "clang-format"}
>> diff --git a/mercurial/cext/revlog.c b/mercurial/cext/revlog.c
>> --- a/mercurial/cext/revlog.c
>> +++ b/mercurial/cext/revlog.c
>> @@ -995,8 +995,8 @@ static inline int index_baserev(indexObj
>> 	if (result < -1) {
>> 		PyErr_Format(
>> 		    PyExc_ValueError,
>> -		    "corrupted revlog, revision base out of range: %d, %d",
>> -		    rev, result);
>> +		    "corrupted revlog, revision base out of range: %d, %d", rev,
>> +		    result);
>> 		return -2;
>> 	}
>> 	return result;
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/.hg-format-source b/.hg-format-source
new file mode 100644
--- /dev/null
+++ b/.hg-format-source
@@ -0,0 +1,1 @@ 
+{"configpaths": [".clang-format", "contrib/clang-format-ignorelist"], "pattern": "set:(**.c or **.cc or **.h) and not \"listfile:contrib/clang-format-ignorelist\"", "tool": "clang-format"}
diff --git a/mercurial/cext/revlog.c b/mercurial/cext/revlog.c
--- a/mercurial/cext/revlog.c
+++ b/mercurial/cext/revlog.c
@@ -995,8 +995,8 @@  static inline int index_baserev(indexObj
 	if (result < -1) {
 		PyErr_Format(
 		    PyExc_ValueError,
-		    "corrupted revlog, revision base out of range: %d, %d",
-		    rev, result);
+		    "corrupted revlog, revision base out of range: %d, %d", rev,
+		    result);
 		return -2;
 	}
 	return result;