From: wagi@monom.org (Daniel Wagner)
To: cocci@systeme.lip6.fr
Subject: [Cocci] Formatting issues
Date: Mon, 29 Jul 2013 10:42:52 +0200 [thread overview]
Message-ID: <51F62B0C.2040906@monom.org> (raw)
In-Reply-To: <20130717155132.GA20499@domone.kolej.mff.cuni.cz>
On 07/17/2013 05:51 PM, Ond?ej B?lka wrote:
> On Wed, Jul 17, 2013 at 05:31:15PM +0200, Julia Lawall wrote:
>>> Another formatting issue I found is triggered by a different rule. I figure
>>> the expression 'E' is written out as one line and the original formatting gets
>>> lost.
>>
>> Yes, there is not much I can do about this one. When you match code
>> into a metavariable, all of the whitespace and comments go away. It
>> doesn't know that you still want them in the place where you add the
>> metavariable back in.
>>
>> If you have a probleme with this, as in your example, you can try to
>> rewrite the semantic patch so that the metavariable is there but
>> does not appear in the - and + code. For example, for your first
>> case below you can write
>>
> I ran to problems with formating when I tried use cocci in glibc.
>
> I think that semantic patch and its formatting are two separate issues.
Thought semantic patching can't just ignore formatting.
> Ideal situation is when project formatting is fully machine specified.
> Then we can apply patch, run formatter and we are done.
In my case, I can use checkpatch.pl to report some errors.
> Otherwise we can try to convince other developers that benefit of refactoring
> is bigger than formatting issues it causes or fix it by hand.
>
> I now try to convince others to use first option as this allows all
> submitted patches have perfect formatting because formatter took care of
> it. This can save few round trips when reviewer wants space after comma
> etc.
The main problem I see is as soon you start modifing the output from
spatch by hand you risk to introduce an error and as soon one does
several rounds of review it one will introduce an error.
For the changes in ConnMan I did applied the semantic patch and then
went over the code base via checkpatch.pl and fixed up the errors.
I don't know if a simple rule like 80 chars max couldn't be added for
the --linux-spacing switch. That was the offender in my case. The
rest of the patch was okay.
cheers,
daniel
next prev parent reply other threads:[~2013-07-29 8:42 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-17 14:56 [Cocci] Formatting issues Daniel Wagner
2013-07-17 15:07 ` Julia Lawall
2013-07-17 15:16 ` Daniel Wagner
2013-07-17 15:31 ` Julia Lawall
2013-07-17 15:41 ` Daniel Wagner
2013-07-17 15:51 ` Ondřej Bílka
2013-07-29 8:42 ` Daniel Wagner [this message]
2013-07-29 8:49 ` Julia Lawall
2013-07-29 9:07 ` Daniel Wagner
2013-07-29 9:14 ` Julia Lawall
2013-07-29 14:58 ` [Cocci] Formatting issue Daniel Wagner
2013-07-29 9:28 ` [Cocci] Formatting issues Ondřej Bílka
2013-07-29 14:59 ` Daniel Wagner
2013-07-29 15:59 ` Ondřej Bílka
2013-07-30 21:06 ` Daniel Wagner
2013-07-28 10:48 ` Julia Lawall
2013-07-29 8:33 ` Daniel Wagner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=51F62B0C.2040906@monom.org \
--to=wagi@monom.org \
--cc=cocci@systeme.lip6.fr \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.