From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Duy Nguyen <pclouds@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>,
Junio C Hamano <gitster@pobox.com>,
Ralf Thielow <ralf.thielow@gmail.com>,
Taufiq Hoven <taufiq.hoven@gmail.com>
Subject: Re: [PATCH 2/3] stripspace: respect repository config
Date: Tue, 22 Nov 2016 17:13:59 +0100 (CET) [thread overview]
Message-ID: <alpine.DEB.2.20.1611221712480.3746@virtualbox> (raw)
In-Reply-To: <CACsJy8D5oBR+vo2B+Ro2Q4SX0CG3jME4Gfs1_6AohccmpNvD0A@mail.gmail.com>
Hi Duy,
On Tue, 22 Nov 2016, Duy Nguyen wrote:
> 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
next prev parent reply other threads:[~2016-11-22 16:14 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-21 14:18 [PATCH 0/3] Fix problems with rebase -i when core.commentchar is defined Johannes Schindelin
2016-11-21 14:18 ` [PATCH 1/3] rebase -i: identify problems with core.commentchar Johannes Schindelin
2016-11-21 18:15 ` Junio C Hamano
2016-11-21 18:24 ` Junio C Hamano
2016-11-21 19:05 ` [PATCH 1/3] rebase -i: highlight " Junio C Hamano
2016-11-21 19:05 ` [PATCH 2/3] stripspace: respect repository config Junio C Hamano
2016-11-21 20:28 ` Junio C Hamano
2016-11-22 16:11 ` Johannes Schindelin
2016-11-21 19:05 ` [PATCH 3/3] rebase -i: handle core.commentChar=auto Junio C Hamano
2016-11-21 20:29 ` Junio C Hamano
2016-11-21 20:25 ` [PATCH 1/3] rebase -i: highlight problems with core.commentchar Junio C Hamano
2016-11-22 16:09 ` Johannes Schindelin
2016-11-22 17:05 ` Junio C Hamano
2016-11-23 11:05 ` Johannes Schindelin
2016-11-21 18:49 ` [PATCH 1/3] rebase -i: identify " Jeff King
2016-11-21 19:12 ` Junio C Hamano
2016-11-21 23:38 ` Jeff King
2016-11-22 16:09 ` Johannes Schindelin
2016-11-21 14:18 ` [PATCH 2/3] stripspace: respect repository config Johannes Schindelin
2016-11-22 10:10 ` Duy Nguyen
2016-11-22 16:13 ` Johannes Schindelin [this message]
2016-11-22 17:10 ` Junio C Hamano
2016-11-22 19:10 ` Junio C Hamano
2016-11-22 19:50 ` Jeff King
2016-11-22 20:24 ` Junio C Hamano
2016-11-22 21:19 ` Jeff King
2016-11-22 21:22 ` Junio C Hamano
2016-11-22 21:43 ` Jeff King
2016-11-22 21:55 ` Junio C Hamano
2016-11-23 0:12 ` Jeff King
2016-11-22 21:24 ` Jeff King
2016-11-21 14:18 ` [PATCH 3/3] rebase -i: handle core.commentChar=auto Johannes Schindelin
2016-11-21 18:26 ` Johannes Sixt
2016-11-21 18:40 ` Junio C Hamano
2016-11-21 18:58 ` Johannes Sixt
2016-11-21 19:07 ` Junio C Hamano
2016-11-21 19:14 ` Johannes Sixt
2016-11-22 16:04 ` Johannes Schindelin
2016-11-22 10:31 ` Duy Nguyen
2016-11-21 16:58 ` [PATCH 0/3] Fix problems with rebase -i when core.commentchar is defined Jacob Keller
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=alpine.DEB.2.20.1611221712480.3746@virtualbox \
--to=johannes.schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=pclouds@gmail.com \
--cc=ralf.thielow@gmail.com \
--cc=taufiq.hoven@gmail.com \
/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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).