Re: [PATCH 0/3] Add a function skip_prefix_if_present()
On Wed, Feb 5, 2014 at 1:55 PM, Michael Haggerty wrote: > * Duy seemed to offer to rewrite his patch series, but I don't think > that it has happened yet. It's really low in my todo list. So if you want to pick it up, please do. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Add a function skip_prefix_if_present()
On Wed, Feb 05, 2014 at 07:55:09AM +0100, Michael Haggerty wrote: > * 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. FWIW, after coming down off my eggnog bender, I think I still prefer René's (my?) approach. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Add a function skip_prefix_if_present()
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 mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] Add a function skip_prefix_if_present()
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. Michael Haggerty (3): Add and use a function skip_prefix_if_present() diff_scoreopt_parse(): use skip_prefix_if_present() rev_is_head(): use skip_prefix() builtin/checkout.c| 4 ++-- builtin/fast-export.c | 2 +- builtin/merge.c | 2 +- builtin/show-branch.c | 15 --- diff.c| 6 +++--- git-compat-util.h | 5 + git.c | 2 +- notes.c | 4 ++-- refs.c| 2 +- wt-status.c | 4 ++-- 10 files changed, 26 insertions(+), 20 deletions(-) -- 1.8.5.3 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html