git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 2/3] stripspace: respect repository config

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Nov 22, 2016, 16:13 UTC
Message-ID
<alpine.DEB.2.20.1611221712480.3746@virtualbox>
In-Reply-To
<CACsJy8D5oBR+vo2B+Ro2Q4SX0CG3jME4Gfs1_6AohccmpNvD0A@mail.gmail.com>
Hi Duy,
On Tue, 22 Nov 2016, Duy Nguyen wrote:
Show 37 quoted lines
> On Mon, Nov 21, 2016 at 9:18 PM, Johannes Schindelin
> <johannes.schindelin@gmx.de> wrote:
> > When eff80a9 (Allow custom "comment char", 2013-01-16) taught the
> > `stripspace` command to respect the config setting `core.commentChar`,
> > it forgot that this variable may be defined in .git/config.
> >
> > So when rebasing interactively with a commentChar defined in the current
> > repository's config, the help text at the bottom of the edit script
> > potentially used an incorrect comment character. This was not only
> > funny-looking, but also resulted in tons of warnings like this one:
> >
> >         Warning: the command isn't recognized in the following line
> >          - #
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  builtin/stripspace.c  | 4 +++-
> >  t/t0030-stripspace.sh | 2 +-
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/builtin/stripspace.c b/builtin/stripspace.c
> > index 15e716e..1e62a00 100644
> > --- a/builtin/stripspace.c
> > +++ b/builtin/stripspace.c
> > @@ -44,8 +44,10 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix)
> >         if (argc)
> >                 usage_with_options(stripspace_usage, options);
> >
> > -       if (mode == STRIP_COMMENTS || mode == COMMENT_LINES)
> > +       if (mode == STRIP_COMMENTS || mode == COMMENT_LINES) {
> > +               setup_git_directory_gently(NULL);
> >                 git_config(git_default_config, NULL);
> > +       }
> 
> This conditional config file reading is a trap for similar bugs to
> happen again. Is there any reason we should not just mark the command
> RUN_SETUP_GENTLY in git.c and call git_config() here unconditionally?

As I plan to slip these patches into Git for Windows v2.11.0, i.e. making this a last-minute hot fix, I want to err on the side of caution. So I'd rather keep this conditional (it might regress on the performance front, or something).

Ciao, Dscho

Previous: Duy NguyenNext: Junio C Hamano
Message 4 of 40 in “Fix problems with rebase -i when core.commentchar is defined”
  1. 0/3 Fix problems with rebase -i when core.commentchar is definedJohannes Schindelin, Nov 21, 2016
  2. 2/3 stripspace: respect repository configJohannes Schindelin, Nov 21, 2016
  3. Duy NguyenNov 22, 2016
  4. Johannes SchindelinNov 22, 2016
  5. Junio C HamanoNov 22, 2016
  6. Junio C HamanoNov 22, 2016
  7. Jeff KingNov 22, 2016
  8. Junio C HamanoNov 22, 2016
  9. Jeff KingNov 22, 2016
  10. Junio C HamanoNov 22, 2016
  11. Jeff KingNov 22, 2016
  12. Junio C HamanoNov 22, 2016
  13. Jeff KingNov 23, 2016
  14. Jeff KingNov 22, 2016
  15. 1/3 rebase -i: identify problems with core.commentcharJohannes Schindelin, Nov 21, 2016
  16. Junio C HamanoNov 21, 2016
  17. Junio C HamanoNov 21, 2016
  18. 1/3 rebase -i: highlight problems with core.commentcharJunio C Hamano, Nov 21, 2016
  19. 2/3 stripspace: respect repository configJunio C Hamano, Nov 21, 2016
  20. Junio C HamanoNov 21, 2016
  21. Johannes SchindelinNov 22, 2016
  22. 3/3 rebase -i: handle core.commentChar=autoJunio C Hamano, Nov 21, 2016
  23. Junio C HamanoNov 21, 2016
  24. Junio C HamanoNov 21, 2016
  25. Johannes SchindelinNov 22, 2016
  26. Junio C HamanoNov 22, 2016
  27. Johannes SchindelinNov 23, 2016
  28. Jeff KingNov 21, 2016
  29. Junio C HamanoNov 21, 2016
  30. Jeff KingNov 21, 2016
  31. Johannes SchindelinNov 22, 2016
  32. 3/3 rebase -i: handle core.commentChar=autoJohannes Schindelin, Nov 21, 2016
  33. Johannes SixtNov 21, 2016
  34. Junio C HamanoNov 21, 2016
  35. Johannes SixtNov 21, 2016
  36. Junio C HamanoNov 21, 2016
  37. Johannes SixtNov 21, 2016
  38. Johannes SchindelinNov 22, 2016
  39. Duy NguyenNov 22, 2016
  40. Jacob KellerNov 21, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.