Re: [PATCH 0/3] Add a function skip_prefix_if_present()

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Feb 5, 2014, 06:55 UTC
Message-ID: <52F1E04D.3090208@alum.mit.edu>
In-Reply-To: <1391581517-8586-1-git-send-email-mhagger@alum.mit.edu>

On 02/05/2014 07:25 AM, Michael Haggerty wrote:

Show 6 quoted lines

> These patches apply on top of gitster/nd/more-skip-prefix.
> 
> I noticed that Duy's new function skip_prefix_defval() was mostly
> being called with its first and third arguments the same.  So
> introduce a function skip_prefix_if_present() that implements this
> pattern.

I see I should have read the whole previous thread [1] before firing off this patch series. What I learned when I read it just now:

* Johannes Sixt didn't think changes like the following improve
  readability:

> -		if (starts_with(arg, "--upload-pack=")) {
> -			args.uploadpack = arg + 14;
> +		if ((optarg = skip_prefix(arg, "--upload-pack=")) != NULL) {
> +			args.uploadpack = optarg;

* René Scharfe submitted a patch to use a function parse_prefix()
  (originally suggested by Peff) instead of Duy's suggested approach:

      http://article.gmane.org/gmane.comp.version-control.git/239569

  His patch appears to have been overlooked.

* Duy seemed to offer to rewrite his patch series, but I don't think
  that it has happened yet.

And then the conversation was drowned by Christmas eggnog.

I don't have a strong feeling about (Duy's proposal plus my patches) vs. (René's parse_prefix() approach). But I definitely *do* like the idea of getting rid of all those awkward magic numbers everywhere.

Michael

[1] http://thread.gmane.org/gmane.comp.version-control.git/239438/focus=239569

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/