On Sun, Oct 28, 2018 at 11:32 AM Martin Ågren <martin.ag...@gmail.com> wrote: > [...] > Let's be explicit about breaking out of the loop. This helps the > compiler grok our intention. As a bonus, it might make it (even) more > obvious to human readers that the loop stops at the first space.
This did come up in review[1,2]. I had a hard time understanding the loop because it looked idiomatic but wasn't (and we have preconceived notions about behavior of things which look idiomatic). [1]: https://public-inbox.org/git/CAPig+cQbG2s-LrAo9+7C7=dXifbWFJ3SzuNa-QePHDk7egK=j...@mail.gmail.com/ [2]: https://public-inbox.org/git/capig+crju6nixpt2frdwz0x1hmgf1ojvzj3uk2qxege-s7i...@mail.gmail.com/ > Signed-off-by: Martin Ågren <martin.ag...@gmail.com> > --- > diff --git a/sequencer.c b/sequencer.c > @@ -2849,10 +2849,14 @@ static int do_reset(const char *name, int len, struct > replay_opts *opts) > /* Determine the length of the label */ > + for (i = 0; i < len; i++) { > + if (isspace(name[i])) { > len = i; > + break; > + } > + } > strbuf_addf(&ref_name, "refs/rewritten/%.*s", len, name); At least for me, this would be more idiomatic if it was coded like this: for (i = 0; i < len; i++) if (isspace(name[i])) break; strbuf_addf(..., i, name); or, perhaps (less concise): for (i = 0; i < len; i++) if (isspace(name[i])) break; len = i; strbuf_addf(..., len, name);
