Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Feb 15, 2016 at 03:58:12PM -0500, Eric Sunshine wrote:

> > diff --git a/builtin/config.c b/builtin/config.c
> > @@ -27,6 +28,7 @@ static int actions, types;
> >  static const char *get_color_slot, *get_colorbool_slot;
> >  static int end_null;
> 
> Not related to your changes, but I just realized that this variable
> really ought to be named 'end_nul' since we're talking about the
> character NUL, not a NULL pointer.

Yeah, I noticed that, too. We just went through a round of related fixes
here, and the "usual" name is now "nul_term_line". I don't especially
like that one either, but at least it is correct and consistent. :)

> >  static int respect_includes = -1;
> > +static int show_origin;
> > @@ -81,6 +83,7 @@ static struct option builtin_config_options[] = {
> >         OPT_BOOL('z', "null", &end_null, N_("terminate values with NUL byte")),
> 
> Likewise, the long option name should be --nul rather than --null, or
> the long name could be dropped altogether since some other commands
> just recognize short option -z.
> 
> There is no need for this patch series to address this anomaly; it's
> perhaps low-hanging fruit for someone wanting to join the project. The
> only very minor wrinkle is that we'd still need to recognize --null as
> a deprecated (and undocumented) alias for --nul.

I think that would be OK as long as we keep the compatible option.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]