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

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

 



On 02/05/2014 07:25 AM, Michael Haggerty wrote:
> 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@xxxxxxxxxxxx
http://softwareswirl.blogspot.com/
--
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]