| Message ID | 20200504160709.GB12842@coredump.intra.peff.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | CodingGuidelines: drop arithmetic expansion advice to use "$x" | expand |
On Mon, May 04, 2020 at 12:07:09PM -0400, Jeff King wrote: > -- >8 -- > Subject: CodingGuidelines: drop arithmetic expansion advice to use "$x" > > The advice to use "$x" rather than "x" in arithmetric expansion was > working around a dash bug fixed in 0.5.4. Even Debian oldstable has > 0.5.7 these days. that would be oldoldstable, oldstable is actually in 0.5.8 ;) > Helped-by: Danh Doan <congdanhqx@gmail.com> > Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Carlo
On Mon, May 04, 2020 at 09:28:44AM -0700, Carlo Marcelo Arenas Belón wrote: > On Mon, May 04, 2020 at 12:07:09PM -0400, Jeff King wrote: > > -- >8 -- > > Subject: CodingGuidelines: drop arithmetic expansion advice to use "$x" > > > > The advice to use "$x" rather than "x" in arithmetric expansion was > > working around a dash bug fixed in 0.5.4. Even Debian oldstable has > > 0.5.7 these days. > > that would be oldoldstable, oldstable is actually in 0.5.8 ;) Oh, you're right. I forgot they're doing an extra layer these days. It will officially become obsolete in less than 2 months. :) -Peff
Jeff King <peff@peff.net> writes: > On Mon, May 04, 2020 at 09:28:44AM -0700, Carlo Marcelo Arenas Belón wrote: > >> On Mon, May 04, 2020 at 12:07:09PM -0400, Jeff King wrote: >> > -- >8 -- >> > Subject: CodingGuidelines: drop arithmetic expansion advice to use "$x" >> > >> > The advice to use "$x" rather than "x" in arithmetric expansion was >> > working around a dash bug fixed in 0.5.4. Even Debian oldstable has >> > 0.5.7 these days. >> >> that would be oldoldstable, oldstable is actually in 0.5.8 ;) > > Oh, you're right. I forgot they're doing an extra layer these days. It > will officially become obsolete in less than 2 months. :) Thanks, both. I'll just do s/0.5.7/0.5.8/ when I add Carlo's reviewed-by to queue the patch.
On 2020-05-04 12:07:09-0400, Jeff King <peff@peff.net> wrote: > On Mon, May 04, 2020 at 08:37:38AM -0700, Junio C Hamano wrote: > > Subject: CodingGuidelines: drop arithmetic expansion advice to use "$x" > > The advice to use "$x" rather than "x" in arithmetric expansion was > working around a dash bug fixed in 0.5.4. Even Debian oldstable has > 0.5.7 these days. And in the meantime, we've added almost two dozen > instances of the "x" form which you can find with: > > git grep '$(([a-z]' > > and nobody seems to have complained. Let's declare this workaround > obsolete and simplify our style guide. > > Helped-by: Danh Doan <congdanhqx@gmail.com> > Signed-off-by: Jeff King <peff@peff.net> I see this patch hasn't been merged to pu yet. Please have my name as (if it's not too much trouble for you): Đoàn Trần Công Danh <congdanhqx@gmail.com> (I'm going to change my name in email setting)
Jeff King <peff@peff.net> writes: > diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines > index 390ceece52..a89e8dcfbc 100644 > --- a/Documentation/CodingGuidelines > +++ b/Documentation/CodingGuidelines > @@ -95,10 +95,6 @@ For shell scripts specifically (not exhaustive): > > - We use Arithmetic Expansion $(( ... )). > > - - Inside Arithmetic Expansion, spell shell variables with $ in front > - of them, as some shells do not grok $((x)) while accepting $(($x)) > - just fine (e.g. dash older than 0.5.4). > - > - We do not use Process Substitution <(list) or >(list). > > - Do not write control structures on a single line with semicolon. A new entry in the "What's cooking" report has this: * jk/arith-expansion-coding-guidelines (2020-05-04) 1 commit - CodingGuidelines: drop arithmetic expansion advice to use "$x" The coding guideline for shell scripts instructed to refer to a variable with dollar-sign inside airthmetic expansion to work around a bug in old versions of bash, which is a thing of the past. Now we are not forbidden from writing $((var+1)). Writing the last sentence made me wonder if we should go one step further and actually encourage actively omitting the dollar-sign from variable reference instead.
On Tue, May 05, 2020 at 01:40:03PM -0700, Junio C Hamano wrote: > A new entry in the "What's cooking" report has this: > > * jk/arith-expansion-coding-guidelines (2020-05-04) 1 commit > - CodingGuidelines: drop arithmetic expansion advice to use "$x" > > The coding guideline for shell scripts instructed to refer to a > variable with dollar-sign inside airthmetic expansion to work > around a bug in old versions of bash, which is a thing of the past. > Now we are not forbidden from writing $((var+1)). s/bash/dash/, I think > Writing the last sentence made me wonder if we should go one step > further and actually encourage actively omitting the dollar-sign > from variable reference instead. I don't have a strong preference either way. I gave a few reasons to prefer the dollar-less version in: https://lore.kernel.org/git/20200504151351.GC11373@coredump.intra.peff.net/ but I couldn't find a case where the difference really matters in practice for otherwise-correct code. If we don't care much either way, I'd just as soon not have a rule. We have enough rules as it is, and I don't think either is obvious enough that somebody who is used to one style will get confused by the other. -Peff
Jeff King <peff@peff.net> writes: > I'd just as soon not have a rule. We have enough rules as it is, and I > don't think either is obvious enough that somebody who is used to one > style will get confused by the other. OK. I very much welcome one fewer paragraphs in the file ;-)
diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index 390ceece52..a89e8dcfbc 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -95,10 +95,6 @@ For shell scripts specifically (not exhaustive): - We use Arithmetic Expansion $(( ... )). - - Inside Arithmetic Expansion, spell shell variables with $ in front - of them, as some shells do not grok $((x)) while accepting $(($x)) - just fine (e.g. dash older than 0.5.4). - - We do not use Process Substitution <(list) or >(list). - Do not write control structures on a single line with semicolon.