Re: [PATCH 1/3] rev-parse: support OPT_NUMBER_CALLBACK in --parseopt

2014-03-04 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy  pclo...@gmail.com writes:

 If the option spec is

 -NUM Help string

 then rev-parse will accept and parse -([0-9]+) and return -NUM $1

Even though the hardcoded NUM token initially gave me a knee-jerk
Yuck reaction, that literal option name is very unlikely to be
desired by scripts/commands for their real option names, and being
in all uppercase it is very clear that it is magic convention
between the parsing mechanism and the script it uses.

It however felt funny to me without a matching (possibly hidden)
mechanism to allow parse-options machinery to consume such an output
as its input.  In a script that uses this mechanism to parse out the
numeric option -NUM 3 out of git script -3 and uses that three
to drive an underlying command (e.g. git grep -3), wouldn't it be
more natural if that underlying command can be told to accept the
same notation (i.e. git grep -NUM 3)?  For that to be consistent
with the rest of the system, -NUM would not be a good token; being
it multi-character, it must be --NUM or something with double-dash
prefix.

I kind of like the basic idea, the capability it tries to give
scripted Porcelain implementations.  But my impression is that
rebase -i -4, which this mechanism was invented for, is not
progressing, so perhaps we should wait until the real user of this
feature appears.

Thanks.

 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
  builtin/rev-parse.c | 9 +++--
  1 file changed, 7 insertions(+), 2 deletions(-)

 diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
 index 45901df..b37676f 100644
 --- a/builtin/rev-parse.c
 +++ b/builtin/rev-parse.c
 @@ -331,6 +331,8 @@ static int parseopt_dump(const struct option *o, const 
 char *arg, int unset)
   struct strbuf *parsed = o-value;
   if (unset)
   strbuf_addf(parsed,  --no-%s, o-long_name);
 + else if (o-type == OPTION_NUMBER)
 + strbuf_addf(parsed,  -NUM);
   else if (o-short_name  (o-long_name == NULL || !stuck_long))
   strbuf_addf(parsed,  -%c, o-short_name);
   else
 @@ -338,7 +340,7 @@ static int parseopt_dump(const struct option *o, const 
 char *arg, int unset)
   if (arg) {
   if (!stuck_long)
   strbuf_addch(parsed, ' ');
 - else if (o-long_name)
 + else if (o-long_name || o-type == OPTION_NUMBER)
   strbuf_addch(parsed, '=');
   sq_quote_buf(parsed, arg);
   }
 @@ -439,7 +441,10 @@ static int cmd_parseopt(int argc, const char **argv, 
 const char *prefix)
  
   if (s - sb.buf == 1) /* short option only */
   o-short_name = *sb.buf;
 - else if (sb.buf[1] != ',') /* long option only */
 + else if (s - sb.buf == 4  !strncmp(sb.buf, -NUM, 4)) {
 + o-type = OPTION_NUMBER;
 + o-flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG;
 + } else if (sb.buf[1] != ',') /* long option only */
   o-long_name = xmemdupz(sb.buf, s - sb.buf);
   else {
   o-short_name = *sb.buf;
--
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 1/3] rev-parse: support OPT_NUMBER_CALLBACK in --parseopt

2014-03-01 Thread Nguyễn Thái Ngọc Duy
If the option spec is

-NUM Help string

then rev-parse will accept and parse -([0-9]+) and return -NUM $1

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 builtin/rev-parse.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 45901df..b37676f 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -331,6 +331,8 @@ static int parseopt_dump(const struct option *o, const char 
*arg, int unset)
struct strbuf *parsed = o-value;
if (unset)
strbuf_addf(parsed,  --no-%s, o-long_name);
+   else if (o-type == OPTION_NUMBER)
+   strbuf_addf(parsed,  -NUM);
else if (o-short_name  (o-long_name == NULL || !stuck_long))
strbuf_addf(parsed,  -%c, o-short_name);
else
@@ -338,7 +340,7 @@ static int parseopt_dump(const struct option *o, const char 
*arg, int unset)
if (arg) {
if (!stuck_long)
strbuf_addch(parsed, ' ');
-   else if (o-long_name)
+   else if (o-long_name || o-type == OPTION_NUMBER)
strbuf_addch(parsed, '=');
sq_quote_buf(parsed, arg);
}
@@ -439,7 +441,10 @@ static int cmd_parseopt(int argc, const char **argv, const 
char *prefix)
 
if (s - sb.buf == 1) /* short option only */
o-short_name = *sb.buf;
-   else if (sb.buf[1] != ',') /* long option only */
+   else if (s - sb.buf == 4  !strncmp(sb.buf, -NUM, 4)) {
+   o-type = OPTION_NUMBER;
+   o-flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG;
+   } else if (sb.buf[1] != ',') /* long option only */
o-long_name = xmemdupz(sb.buf, s - sb.buf);
else {
o-short_name = *sb.buf;
-- 
1.9.0.40.gaa8c3ea

--
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