subject:"\[PATCH 3\/5\] diff\: acknowledge \-\-submodule=short command\-line option"

Re: [PATCH 3/5] diff: acknowledge --submodule=short command-line option

2012-10-07 Thread Ramkumar Ramachandra

Jens Lehmann wrote:
 Am 02.10.2012 18:51, schrieb Ramkumar Ramachandra:
 Currently, the diff code does not differentiate between an explicit
 '--submodule=short' being passed, and no submodule option being passed
 on the command line.  Making this differentiation will be important
 when the command-line option can be used to override a
 diff.submoduleFormat configuration variable introduced in the next
 patch.

 Wouldn't it be sufficient here to simply reset the log flag by using
 DIFF_OPT_CLR(options, SUBMODULE_LOG)? This would avoid having to
 use the last bit of the diffopt flags. And if I read the code correctly,
 diff_opt_parse() is called by setup_revisions() which is called after
 git_config(), so that should be safe. (And textconv uses the same
 approach)

How is it sufficient?  In git_diff_ui_config(), I set
submodule_format_cfg, which has nothing to do with SUBMODULE_LOG.  In
builtin_diff(), I'll have to check SUBMODULE_LOG and
submodule_format_cfg.  The tricky bit is that I should check
submodule_format_cfg if and only if --submodule=short was NOT passed
on the command line-  now, that's not the same thing is checking if
SUBMODULE_LOG is unset, because SUBMODULE_LOG is unset (or cleared) if
no argument was passed or if --submodule=short is passed.
Therefore, I need a SUBMODULE_SHORT to differentiate between the two
cases.

What am I missing?

Ram
--
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 3/5] diff: acknowledge --submodule=short command-line option

2012-10-07 Thread Jens Lehmann

Am 07.10.2012 17:22, schrieb Ramkumar Ramachandra:
 Jens Lehmann wrote:
 Am 02.10.2012 18:51, schrieb Ramkumar Ramachandra:
 Currently, the diff code does not differentiate between an explicit
 '--submodule=short' being passed, and no submodule option being passed
 on the command line.  Making this differentiation will be important
 when the command-line option can be used to override a
 diff.submoduleFormat configuration variable introduced in the next
 patch.

 Wouldn't it be sufficient here to simply reset the log flag by using
 DIFF_OPT_CLR(options, SUBMODULE_LOG)? This would avoid having to
 use the last bit of the diffopt flags. And if I read the code correctly,
 diff_opt_parse() is called by setup_revisions() which is called after
 git_config(), so that should be safe. (And textconv uses the same
 approach)
 
 How is it sufficient?  In git_diff_ui_config(), I set
 submodule_format_cfg, which has nothing to do with SUBMODULE_LOG.  In
 builtin_diff(), I'll have to check SUBMODULE_LOG and
 submodule_format_cfg.  The tricky bit is that I should check
 submodule_format_cfg if and only if --submodule=short was NOT passed
 on the command line-  now, that's not the same thing is checking if
 SUBMODULE_LOG is unset, because SUBMODULE_LOG is unset (or cleared) if
 no argument was passed or if --submodule=short is passed.
 Therefore, I need a SUBMODULE_SHORT to differentiate between the two
 cases.
 
 What am I missing?

I forgot to mention that testing submodule_format_cfg would have to
happen in cmd_diff() (between reading the config and parsing the
command line options) instead of builtin_diff(). Something like this
should do the trick (untested):

diff --git a/builtin/diff.c b/builtin/diff.c
index 9650be2..180bf44 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -297,6 +297,9 @@ int cmd_diff(int argc, const char **argv, const char *prefix
DIFF_OPT_SET(rev.diffopt, ALLOW_EXTERNAL);
DIFF_OPT_SET(rev.diffopt, ALLOW_TEXTCONV);

+   if (submodule_format_cfg  !strcmp(submodule_format_cfg, log))
+   DIFF_OPT_SET(options, SUBMODULE_LOG);
+
if (nongit)
die(_(Not a git repository));
argc = setup_revisions(argc, argv, rev, NULL);

--
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 3/5] diff: acknowledge --submodule=short command-line option

2012-10-02 Thread Ramkumar Ramachandra

Currently, the diff code does not differentiate between an explicit
'--submodule=short' being passed, and no submodule option being passed
on the command line.  Making this differentiation will be important
when the command-line option can be used to override a
diff.submoduleFormat configuration variable introduced in the next
patch.

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 diff.c |4 +++-
 diff.h |   17 +
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/diff.c b/diff.c
index 35d3f07..8ea40f9 100644
--- a/diff.c
+++ b/diff.c
@@ -3647,7 +3647,9 @@ int diff_opt_parse(struct diff_options *options, const 
char **av, int ac)
} else if (!strcmp(arg, --submodule))
DIFF_OPT_SET(options, SUBMODULE_LOG);
else if (!prefixcmp(arg, --submodule=)) {
-   if (!strcmp(arg + 12, log))
+   if (!strcmp(arg + 12, short))
+   DIFF_OPT_SET(options, SUBMODULE_SHORT);
+   else if (!strcmp(arg + 12, log))
DIFF_OPT_SET(options, SUBMODULE_LOG);
}
 
diff --git a/diff.h b/diff.h
index a658f85..4115b49 100644
--- a/diff.h
+++ b/diff.h
@@ -77,14 +77,15 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct 
diff_options *opt, void *data)
 #define DIFF_OPT_DIRSTAT_BY_FILE (1  20)
 #define DIFF_OPT_ALLOW_TEXTCONV  (1  21)
 #define DIFF_OPT_DIFF_FROM_CONTENTS  (1  22)
-#define DIFF_OPT_SUBMODULE_LOG   (1  23)
-#define DIFF_OPT_DIRTY_SUBMODULES(1  24)
-#define DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES (1  25)
-#define DIFF_OPT_IGNORE_DIRTY_SUBMODULES (1  26)
-#define DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG (1  27)
-#define DIFF_OPT_DIRSTAT_BY_LINE (1  28)
-#define DIFF_OPT_FUNCCONTEXT (1  29)
-#define DIFF_OPT_PICKAXE_IGNORE_CASE (1  30)
+#define DIFF_OPT_SUBMODULE_SHORT (1  23)
+#define DIFF_OPT_SUBMODULE_LOG   (1  24)
+#define DIFF_OPT_DIRTY_SUBMODULES(1  25)
+#define DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES (1  26)
+#define DIFF_OPT_IGNORE_DIRTY_SUBMODULES (1  27)
+#define DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG (1  28)
+#define DIFF_OPT_DIRSTAT_BY_LINE (1  29)
+#define DIFF_OPT_FUNCCONTEXT (1  30)
+#define DIFF_OPT_PICKAXE_IGNORE_CASE (1  31)
 
 #define DIFF_OPT_TST(opts, flag)((opts)-flags  DIFF_OPT_##flag)
 #define DIFF_OPT_SET(opts, flag)((opts)-flags |= DIFF_OPT_##flag)
-- 
1.7.8.1.362.g5d6df.dirty

--
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 3/5] diff: acknowledge --submodule=short command-line option

2012-10-02 Thread Jens Lehmann

Am 02.10.2012 18:51, schrieb Ramkumar Ramachandra:
 Currently, the diff code does not differentiate between an explicit
 '--submodule=short' being passed, and no submodule option being passed
 on the command line.  Making this differentiation will be important
 when the command-line option can be used to override a
 diff.submoduleFormat configuration variable introduced in the next
 patch.

Wouldn't it be sufficient here to simply reset the log flag by using
DIFF_OPT_CLR(options, SUBMODULE_LOG)? This would avoid having to
use the last bit of the diffopt flags. And if I read the code correctly,
diff_opt_parse() is called by setup_revisions() which is called after
git_config(), so that should be safe. (And textconv uses the same
approach)

 Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
 ---
  diff.c |4 +++-
  diff.h |   17 +
  2 files changed, 12 insertions(+), 9 deletions(-)
 
 diff --git a/diff.c b/diff.c
 index 35d3f07..8ea40f9 100644
 --- a/diff.c
 +++ b/diff.c
 @@ -3647,7 +3647,9 @@ int diff_opt_parse(struct diff_options *options, const 
 char **av, int ac)
   } else if (!strcmp(arg, --submodule))
   DIFF_OPT_SET(options, SUBMODULE_LOG);
   else if (!prefixcmp(arg, --submodule=)) {
 - if (!strcmp(arg + 12, log))
 + if (!strcmp(arg + 12, short))
 + DIFF_OPT_SET(options, SUBMODULE_SHORT);
 + else if (!strcmp(arg + 12, log))
   DIFF_OPT_SET(options, SUBMODULE_LOG);
   }
  
 diff --git a/diff.h b/diff.h
 index a658f85..4115b49 100644
 --- a/diff.h
 +++ b/diff.h
 @@ -77,14 +77,15 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct 
 diff_options *opt, void *data)
  #define DIFF_OPT_DIRSTAT_BY_FILE (1  20)
  #define DIFF_OPT_ALLOW_TEXTCONV  (1  21)
  #define DIFF_OPT_DIFF_FROM_CONTENTS  (1  22)
 -#define DIFF_OPT_SUBMODULE_LOG   (1  23)
 -#define DIFF_OPT_DIRTY_SUBMODULES(1  24)
 -#define DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES (1  25)
 -#define DIFF_OPT_IGNORE_DIRTY_SUBMODULES (1  26)
 -#define DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG (1  27)
 -#define DIFF_OPT_DIRSTAT_BY_LINE (1  28)
 -#define DIFF_OPT_FUNCCONTEXT (1  29)
 -#define DIFF_OPT_PICKAXE_IGNORE_CASE (1  30)
 +#define DIFF_OPT_SUBMODULE_SHORT (1  23)
 +#define DIFF_OPT_SUBMODULE_LOG   (1  24)
 +#define DIFF_OPT_DIRTY_SUBMODULES(1  25)
 +#define DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES (1  26)
 +#define DIFF_OPT_IGNORE_DIRTY_SUBMODULES (1  27)
 +#define DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG (1  28)
 +#define DIFF_OPT_DIRSTAT_BY_LINE (1  29)
 +#define DIFF_OPT_FUNCCONTEXT (1  30)
 +#define DIFF_OPT_PICKAXE_IGNORE_CASE (1  31)
  
  #define DIFF_OPT_TST(opts, flag)((opts)-flags  DIFF_OPT_##flag)
  #define DIFF_OPT_SET(opts, flag)((opts)-flags |= DIFF_OPT_##flag)
 

--
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 3/5] diff: acknowledge --submodule=short command-line option

Re: [PATCH 3/5] diff: acknowledge --submodule=short command-line option

[PATCH 3/5] diff: acknowledge --submodule=short command-line option

Re: [PATCH 3/5] diff: acknowledge --submodule=short command-line option

4 matches

Site Navigation

Mail list logo

Footer information