From: Eric Sunshine <firstname.lastname@example.org> To: "Martin Ågren" <email@example.com> Cc: Git List <firstname.lastname@example.org>, Junio C Hamano <email@example.com>, Chris Torek <firstname.lastname@example.org> Subject: Re: [PATCH v2 1/2] t: don't spuriously close and reopen quotes Date: Fri, 7 Aug 2020 12:17:22 -0400 Message-ID: <CAPig+cR+JmQsj9qAALq6oxYQb9E94TAEqXHx+dAt=E0FZH6WwA@mail.gmail.com> (raw) In-Reply-To: <CAN0heSpjgc0GUHnebeBdtr6Yny3Y_jsjz5hTfJqw15bZiPc8HQ@mail.gmail.com> On Fri, Aug 7, 2020 at 4:45 AM Martin Ågren <email@example.com> wrote: > On Thu, 6 Aug 2020 at 22:26, Eric Sunshine <firstname.lastname@example.org> wrote: > > On Thu, Aug 6, 2020 at 4:09 PM Martin Ågren <email@example.com> wrote: > > > - echo "$fifth branch 'fifth' of ." | > > > + echo "$fifth branch fifth of ." | > > > > This one is a bit weird. It really seems as if the intent was to quote > > the word "fifth" in the merge message, so dropping the quotes > > altogether seems wrong. However, the file 'msg' is never even > > consulted in this test (or any other test), so is this just "dead > > code" (including the leading 'fifth=' assignment which also is > > otherwise unused outside the 'echo')? > > Huh, good catch. [...] So I should be able to safely drop this > "dead code" entirely. That could be done atop this series if there is no other reason to re-roll. > > > - git tag -a -m 'annotated' anno HEAD && > > > + git tag -a -m "annotated" anno HEAD && > > > > There are a fair number of these quoted single-token arguments > > containing no special characters which don't actually need to be > > quoted at all, so an alternative would be simply to drop the quotes > > altogether, making the commands less syntactically noisy. However, > > that might be outside the intended scope of this change. > > If we say that "don't use quotes if you don't need to" is a reasonable > thing to strive for, I can drop these in a reroll. I think I'll be > injecting a patch anyway for the "msg" you pointed out in t4200, so I > can certainly tweak this patch to be a bit more aggressive in dropping > unnecessary quotes. No need. Matching the local convention makes sense, and I don't insist upon such a change at all. Mine was a non-actionable observation, and it's entirely subjective anyhow. I don't feel strongly about whether the series should be re-rolled. It's true that dropping that dead code (mentioned above) would make more sense coming before the patch which fixes up the quoting, but it wouldn't bother me if the dead-code removal was done as a follow-on patch.
next prev parent reply other threads:[~2020-08-07 16:17 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-08-01 22:06 [PATCH] t1450: fix quoting of NUL byte when corrupting pack Martin Ågren 2020-08-02 0:45 ` Junio C Hamano 2020-08-02 14:30 ` Martin Ågren 2020-08-02 17:22 ` Eric Sunshine 2020-08-06 20:08 ` [PATCH v2 0/2] t: don't spuriously close and reopen quotes Martin Ågren 2020-08-06 20:08 ` [PATCH v2 1/2] " Martin Ågren 2020-08-06 20:26 ` Eric Sunshine 2020-08-07 8:45 ` Martin Ågren 2020-08-07 16:17 ` Eric Sunshine [this message] 2020-08-07 17:16 ` Junio C Hamano 2020-08-06 20:08 ` [PATCH v2 2/2] t4104: modernize and simplify quoting Martin Ågren 2020-08-02 1:00 ` [PATCH] t1450: fix quoting of NUL byte when corrupting pack Chris Torek 2020-08-02 1:02 ` Chris Torek 2020-08-02 14:35 ` Martin Ågren 2020-08-02 16:20 ` Chris Torek 2020-08-02 17:57 ` Junio C Hamano
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 List information: http://vger.kernel.org/majordomo-info.html * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAPig+cR+JmQsj9qAALq6oxYQb9E94TAEqXHx+dAt=E0FZH6WwA@mail.gmail.com' \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ /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
email@example.com list mirror (unofficial, one of many) This inbox may be cloned and mirrored by anyone: git clone --mirror http://public-inbox.org/git git clone --mirror http://ou63pmih66umazou.onion/git git clone --mirror http://czquwvybam4bgbro.onion/git git clone --mirror http://hjrcffqmbrq6wope.onion/git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V1 git git/ http://public-inbox.org/git \ firstname.lastname@example.org public-inbox-index git Example config snippet for mirrors. Newsgroups are available over NNTP: nntp://news.public-inbox.org/inbox.comp.version-control.git nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git nntp://news.gmane.io/gmane.comp.version-control.git note: .onion URLs require Tor: https://www.torproject.org/ code repositories for the project(s) associated with this inbox: https://80x24.org/mirrors/git.git AGPL code for this site: git clone https://public-inbox.org/public-inbox.git