Re: [RFC/PATCH 1/9] ref-filter: add %(refname:lalignX) option
On Sun, Jun 28, 2015 at 1:09 PM, Junio C Hamano gits...@pobox.com wrote: Karthik Nayak karthik@gmail.com writes: Add support for %(refname:lalignX) where X is a number. This will print a shortened refname aligned to the left followed by spaces for a total length of X characters. If X is less than the shortened refname size, the entire shortened refname is printed. Why would we even want this kind of thing in the first place? Is this to make it possible to re-implement some option that exists already in 'git branch' or 'git tag' as a thin wrapper on top of this underlying feature? As a low-level plumbing, I'd rather not to see such an elaborate formatting option added to for-each-ref; after all, the design of the command to allow its output properly script-quoted is so that we can offload such non-essential (meaning: does not need anything only Git knows; computing the display width of a string and filling the output space is an example. As opposed to something like --merged that needs help from Git, which has the ultimate knowledge on the topology) processing to Porcelain that uses the command as plumbing. Well this is to support the printing of tag in `git tag -l`, if you see the current implementation we print refs using printf(%-15s , refname); whenever the -n[n] option is given, hence this patch to support printing of refs using a required spacing. I couldn't find a way to do with the current options available in for-each-ref/ref-filter. Is there a better way to go about this? -- Regards, Karthik Nayak -- 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: [RFC/PATCH 1/9] ref-filter: add %(refname:lalignX) option
Hey Christian, On Sun, Jun 28, 2015 at 1:32 AM, Christian Couder christian.cou...@gmail.com wrote: After thinking about such code, I wonder if it would be better to support %(refname:lalign=X) instead of %(refname:lalignX). The reason why it might be interesting to require an = sign between align and the number X is that if we later want to introduce another option with a name that starts with lalign, for example %(refname:lalignall=X) that would truncate the refname if it is bigger than X), we might be more backward compatible with old git versions that implement %(refname:lalign=X) but not %(refname:lalignall=X). We will be more backward compatible because the above call to starts_with() would probably be something like: if (starts_with(formatp, lalign=)) { which means that old git versions would ignore something like lalignall=X. Good point! I agree with what you said, including an = sign would mean more compatibility in the future as we could have lalign* options. Will change this, thanks. -- Regards, Karthik Nayak -- 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: [RFC/PATCH 1/9] ref-filter: add %(refname:lalignX) option
Karthik Nayak karthik@gmail.com writes: Add support for %(refname:lalignX) where X is a number. This will print a shortened refname aligned to the left followed by spaces for a total length of X characters. If X is less than the shortened refname size, the entire shortened refname is printed. Why would we even want this kind of thing in the first place? Is this to make it possible to re-implement some option that exists already in 'git branch' or 'git tag' as a thin wrapper on top of this underlying feature? As a low-level plumbing, I'd rather not to see such an elaborate formatting option added to for-each-ref; after all, the design of the command to allow its output properly script-quoted is so that we can offload such non-essential (meaning: does not need anything only Git knows; computing the display width of a string and filling the output space is an example. As opposed to something like --merged that needs help from Git, which has the ultimate knowledge on the topology) processing to Porcelain that uses the command as plumbing. -- 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: [RFC/PATCH 1/9] ref-filter: add %(refname:lalignX) option
Karthik Nayak karthik@gmail.com writes: On Sun, Jun 28, 2015 at 1:09 PM, Junio C Hamano gits...@pobox.com wrote: Why would we even want this kind of thing in the first place? Is this to make it possible to re-implement some option that exists already in 'git branch' or 'git tag' as a thin wrapper on top of this underlying feature? ... Well this is to support the printing of tag in `git tag -l`,... OK, then. Thanks. -- 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: [RFC/PATCH 1/9] ref-filter: add %(refname:lalignX) option
On Thu, Jun 25, 2015 at 1:43 PM, Karthik Nayak karthik@gmail.com wrote: Add support for %(refname:lalignX) where X is a number. This will print a shortened refname aligned to the left followed by spaces for a total length of X characters. If X is less than the shortened refname size, the entire shortened refname is printed. Mentored-by: Christian Couder christian.cou...@gmail.com Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr Signed-off-by: Karthik Nayak karthik@gmail.com --- ref-filter.c | 17 - 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/ref-filter.c b/ref-filter.c index 00d06bf..299b048 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -695,7 +695,22 @@ static void populate_value(struct ref_array_item *ref) int num_ours, num_theirs; formatp++; - if (!strcmp(formatp, short)) + if (starts_with(formatp, lalign)) { + const char *valp; + int val; + + skip_prefix(formatp, lalign, valp); + val = atoi(valp); After thinking about such code, I wonder if it would be better to support %(refname:lalign=X) instead of %(refname:lalignX). The reason why it might be interesting to require an = sign between align and the number X is that if we later want to introduce another option with a name that starts with lalign, for example %(refname:lalignall=X) that would truncate the refname if it is bigger than X), we might be more backward compatible with old git versions that implement %(refname:lalign=X) but not %(refname:lalignall=X). We will be more backward compatible because the above call to starts_with() would probably be something like: if (starts_with(formatp, lalign=)) { which means that old git versions would ignore something like lalignall=X. + refname = shorten_unambiguous_ref(refname, + warn_ambiguous_refs); + if (val strlen(refname)) { + struct strbuf buf = STRBUF_INIT; + strbuf_addstr(buf, refname); + strbuf_addchars(buf, ' ', val - strlen(refname)); + free((char *)refname); + refname = strbuf_detach(buf, NULL); + } Thanks, Christian. -- 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: [RFC/PATCH 1/9] ref-filter: add %(refname:lalignX) option
On Thu, Jun 25, 2015 at 6:43 PM, Karthik Nayak karthik@gmail.com wrote: Add support for %(refname:lalignX) where X is a number. This will print a shortened refname aligned to the left followed by spaces for a total length of X characters. If X is less than the shortened refname size, the entire shortened refname is printed. Mentored-by: Christian Couder christian.cou...@gmail.com Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr Signed-off-by: Karthik Nayak karthik@gmail.com --- ref-filter.c | 17 - 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/ref-filter.c b/ref-filter.c index 00d06bf..299b048 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -695,7 +695,22 @@ static void populate_value(struct ref_array_item *ref) int num_ours, num_theirs; formatp++; - if (!strcmp(formatp, short)) + if (starts_with(formatp, lalign)) { + const char *valp; + int val; + + skip_prefix(formatp, lalign, valp); + val = atoi(valp); + refname = shorten_unambiguous_ref(refname, + warn_ambiguous_refs); + if (val strlen(refname)) { + struct strbuf buf = STRBUF_INIT; + strbuf_addstr(buf, refname); + strbuf_addchars(buf, ' ', val - strlen(refname)); We don't forbid non-ascii characters in refname so you probably want to use utf8_width() here instead of strlen() + free((char *)refname); + refname = strbuf_detach(buf, NULL); + } + } else if (!strcmp(formatp, short)) refname = shorten_unambiguous_ref(refname, warn_ambiguous_refs); else if (!strcmp(formatp, track) -- 2.4.4 -- 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 -- 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: [RFC/PATCH 1/9] ref-filter: add %(refname:lalignX) option
On Sat, Jun 27, 2015 at 10:02 PM, Christian Couder christian.cou...@gmail.com wrote: On Thu, Jun 25, 2015 at 1:43 PM, Karthik Nayak karthik@gmail.com wrote: + if (starts_with(formatp, lalign)) { + const char *valp; + int val; + + skip_prefix(formatp, lalign, valp); + val = atoi(valp); After thinking about such code, I wonder if it would be better to support %(refname:lalign=X) instead of %(refname:lalignX). The reason why it might be interesting to require an = sign between align and the number X is that if we later want to introduce another option with a name that starts with lalign, for example %(refname:lalignall=X) that would truncate the refname if it is bigger than X), we might be more backward compatible with old git versions that implement %(refname:lalign=X) but not %(refname:lalignall=X). We will be more backward compatible because the above call to starts_with() would probably be something like: if (starts_with(formatp, lalign=)) { which means that old git versions would ignore something like lalignall=X. Another reason is that it would be simpler if we ever want to have arbitrary string parameters, like %(refname:substitute=%/%\%). -- 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
[RFC/PATCH 1/9] ref-filter: add %(refname:lalignX) option
Add support for %(refname:lalignX) where X is a number. This will print a shortened refname aligned to the left followed by spaces for a total length of X characters. If X is less than the shortened refname size, the entire shortened refname is printed. Mentored-by: Christian Couder christian.cou...@gmail.com Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr Signed-off-by: Karthik Nayak karthik@gmail.com --- ref-filter.c | 17 - 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/ref-filter.c b/ref-filter.c index 00d06bf..299b048 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -695,7 +695,22 @@ static void populate_value(struct ref_array_item *ref) int num_ours, num_theirs; formatp++; - if (!strcmp(formatp, short)) + if (starts_with(formatp, lalign)) { + const char *valp; + int val; + + skip_prefix(formatp, lalign, valp); + val = atoi(valp); + refname = shorten_unambiguous_ref(refname, + warn_ambiguous_refs); + if (val strlen(refname)) { + struct strbuf buf = STRBUF_INIT; + strbuf_addstr(buf, refname); + strbuf_addchars(buf, ' ', val - strlen(refname)); + free((char *)refname); + refname = strbuf_detach(buf, NULL); + } + } else if (!strcmp(formatp, short)) refname = shorten_unambiguous_ref(refname, warn_ambiguous_refs); else if (!strcmp(formatp, track) -- 2.4.4 -- 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