Re: [RFC/PATCH 1/9] ref-filter: add %(refname:lalignX) option

2015-06-28 Thread Karthik Nayak
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

2015-06-28 Thread Karthik Nayak
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

2015-06-28 Thread Junio C Hamano
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

2015-06-28 Thread Junio C Hamano
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

2015-06-27 Thread Christian Couder
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

2015-06-27 Thread Duy Nguyen
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

2015-06-27 Thread Christian Couder
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

2015-06-25 Thread Karthik Nayak
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