Re: Re: [RFC PATCH] disable complete ignorance of submodules for index - HEAD diff

2013-11-27 Thread Heiko Voigt
On Mon, Nov 25, 2013 at 03:01:34PM +0600, Sergey Sharybin wrote:
 Tested the patch. `git status` now shows the changes to the
 submodules, which is nice :)
 
 However, is it possible to make it so `git commit` lists submodules in
 changes to be committed section, so you'll see what's gonna to be in
 the commit while typing the commit message as well?

Yes, of course that should be shown. Will add in the next iteration.
Which will hopefully be a much simpler implementation. Possibly getting
rid of this new flag.

Cheers Heiko
--
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] disable complete ignorance of submodules for index - HEAD diff

2013-11-25 Thread Sergey Sharybin
Hi,

Tested the patch. `git status` now shows the changes to the
submodules, which is nice :)

However, is it possible to make it so `git commit` lists submodules in
changes to be committed section, so you'll see what's gonna to be in
the commit while typing the commit message as well?

On Sat, Nov 23, 2013 at 7:11 AM, Heiko Voigt hvo...@hvoigt.net wrote:
 If the value of ignore for submodules is set to all we would not show
 whats actually committed during status or diff. This can result in the
 user committing unexpected submodule references. Lets be nicer and always
 show whats in the index.

 Signed-off-by: Heiko Voigt hvo...@hvoigt.net
 ---
 This probably needs splitting up into two patches one for the
 refactoring and one for the actual fix. It is also missing tests, but I
 would first like to know what you think about this approach.

  builtin/diff.c | 43 +++
  diff.h |  2 +-
  submodule.c|  6 --
  wt-status.c|  3 +++
  4 files changed, 35 insertions(+), 19 deletions(-)

 diff --git a/builtin/diff.c b/builtin/diff.c
 index adb93a9..e9a356c 100644
 --- a/builtin/diff.c
 +++ b/builtin/diff.c
 @@ -249,6 +249,21 @@ static int builtin_diff_files(struct rev_info *revs, int 
 argc, const char **argv
 return run_diff_files(revs, options);
  }

 +static int have_cached_option(int argc, const char **argv)
 +{
 +   int i;
 +   for (i = 1; i  argc; i++) {
 +   const char *arg = argv[i];
 +   if (!strcmp(arg, --))
 +   return 0;
 +   else if (!strcmp(arg, --cached) ||
 +!strcmp(arg, --staged)) {
 +   return 1;
 +   }
 +   }
 +   return 0;
 +}
 +
  int cmd_diff(int argc, const char **argv, const char *prefix)
  {
 int i;
 @@ -259,6 +274,7 @@ int cmd_diff(int argc, const char **argv, const char 
 *prefix)
 struct blobinfo blob[2];
 int nongit;
 int result = 0;
 +   int have_cached;

 /*
  * We could get N tree-ish in the rev.pending_objects list.
 @@ -305,6 +321,11 @@ int cmd_diff(int argc, const char **argv, const char 
 *prefix)

 if (nongit)
 die(_(Not a git repository));
 +
 +   have_cached = have_cached_option(argc, argv);
 +   if (have_cached)
 +   DIFF_OPT_SET(rev.diffopt, NO_IGNORE_SUBMODULE);
 +
 argc = setup_revisions(argc, argv, rev, NULL);
 if (!rev.diffopt.output_format) {
 rev.diffopt.output_format = DIFF_FORMAT_PATCH;
 @@ -319,22 +340,12 @@ int cmd_diff(int argc, const char **argv, const char 
 *prefix)
  * Do we have --cached and not have a pending object, then
  * default to HEAD by hand.  Eek.
  */
 -   if (!rev.pending.nr) {
 -   int i;
 -   for (i = 1; i  argc; i++) {
 -   const char *arg = argv[i];
 -   if (!strcmp(arg, --))
 -   break;
 -   else if (!strcmp(arg, --cached) ||
 -!strcmp(arg, --staged)) {
 -   add_head_to_pending(rev);
 -   if (!rev.pending.nr) {
 -   struct tree *tree;
 -   tree = 
 lookup_tree(EMPTY_TREE_SHA1_BIN);
 -   add_pending_object(rev, 
 tree-object, HEAD);
 -   }
 -   break;
 -   }
 +   if (!rev.pending.nr  have_cached) {
 +   add_head_to_pending(rev);
 +   if (!rev.pending.nr) {
 +   struct tree *tree;
 +   tree = lookup_tree(EMPTY_TREE_SHA1_BIN);
 +   add_pending_object(rev, tree-object, HEAD);
 }
 }

 diff --git a/diff.h b/diff.h
 index e342325..81561b3 100644
 --- a/diff.h
 +++ b/diff.h
 @@ -64,7 +64,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct 
 diff_options *opt, void *data)
  #define DIFF_OPT_FIND_COPIES_HARDER  (1   6)
  #define DIFF_OPT_FOLLOW_RENAMES  (1   7)
  #define DIFF_OPT_RENAME_EMPTY(1   8)
 -/* (1   9) unused */
 +#define DIFF_OPT_NO_IGNORE_SUBMODULE (1   9)
  #define DIFF_OPT_HAS_CHANGES (1  10)
  #define DIFF_OPT_QUICK   (1  11)
  #define DIFF_OPT_NO_INDEX(1  12)
 diff --git a/submodule.c b/submodule.c
 index 1905d75..9d81712 100644
 --- a/submodule.c
 +++ b/submodule.c
 @@ -301,9 +301,11 @@ void handle_ignore_submodules_arg(struct diff_options 
 *diffopt,
 DIFF_OPT_CLR(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
 DIFF_OPT_CLR(diffopt, IGNORE_DIRTY_SUBMODULES);

 -   if (!strcmp(arg, all))
 +   if (!strcmp(arg, all)) {
 +   if (DIFF_OPT_TST(diffopt, NO_IGNORE_SUBMODULE))
 +   return;
 

[RFC PATCH] disable complete ignorance of submodules for index - HEAD diff

2013-11-22 Thread Heiko Voigt
If the value of ignore for submodules is set to all we would not show
whats actually committed during status or diff. This can result in the
user committing unexpected submodule references. Lets be nicer and always
show whats in the index.

Signed-off-by: Heiko Voigt hvo...@hvoigt.net
---
This probably needs splitting up into two patches one for the
refactoring and one for the actual fix. It is also missing tests, but I
would first like to know what you think about this approach.

 builtin/diff.c | 43 +++
 diff.h |  2 +-
 submodule.c|  6 --
 wt-status.c|  3 +++
 4 files changed, 35 insertions(+), 19 deletions(-)

diff --git a/builtin/diff.c b/builtin/diff.c
index adb93a9..e9a356c 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -249,6 +249,21 @@ static int builtin_diff_files(struct rev_info *revs, int 
argc, const char **argv
return run_diff_files(revs, options);
 }
 
+static int have_cached_option(int argc, const char **argv)
+{
+   int i;
+   for (i = 1; i  argc; i++) {
+   const char *arg = argv[i];
+   if (!strcmp(arg, --))
+   return 0;
+   else if (!strcmp(arg, --cached) ||
+!strcmp(arg, --staged)) {
+   return 1;
+   }
+   }
+   return 0;
+}
+
 int cmd_diff(int argc, const char **argv, const char *prefix)
 {
int i;
@@ -259,6 +274,7 @@ int cmd_diff(int argc, const char **argv, const char 
*prefix)
struct blobinfo blob[2];
int nongit;
int result = 0;
+   int have_cached;
 
/*
 * We could get N tree-ish in the rev.pending_objects list.
@@ -305,6 +321,11 @@ int cmd_diff(int argc, const char **argv, const char 
*prefix)
 
if (nongit)
die(_(Not a git repository));
+
+   have_cached = have_cached_option(argc, argv);
+   if (have_cached)
+   DIFF_OPT_SET(rev.diffopt, NO_IGNORE_SUBMODULE);
+
argc = setup_revisions(argc, argv, rev, NULL);
if (!rev.diffopt.output_format) {
rev.diffopt.output_format = DIFF_FORMAT_PATCH;
@@ -319,22 +340,12 @@ int cmd_diff(int argc, const char **argv, const char 
*prefix)
 * Do we have --cached and not have a pending object, then
 * default to HEAD by hand.  Eek.
 */
-   if (!rev.pending.nr) {
-   int i;
-   for (i = 1; i  argc; i++) {
-   const char *arg = argv[i];
-   if (!strcmp(arg, --))
-   break;
-   else if (!strcmp(arg, --cached) ||
-!strcmp(arg, --staged)) {
-   add_head_to_pending(rev);
-   if (!rev.pending.nr) {
-   struct tree *tree;
-   tree = lookup_tree(EMPTY_TREE_SHA1_BIN);
-   add_pending_object(rev, tree-object, 
HEAD);
-   }
-   break;
-   }
+   if (!rev.pending.nr  have_cached) {
+   add_head_to_pending(rev);
+   if (!rev.pending.nr) {
+   struct tree *tree;
+   tree = lookup_tree(EMPTY_TREE_SHA1_BIN);
+   add_pending_object(rev, tree-object, HEAD);
}
}
 
diff --git a/diff.h b/diff.h
index e342325..81561b3 100644
--- a/diff.h
+++ b/diff.h
@@ -64,7 +64,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct 
diff_options *opt, void *data)
 #define DIFF_OPT_FIND_COPIES_HARDER  (1   6)
 #define DIFF_OPT_FOLLOW_RENAMES  (1   7)
 #define DIFF_OPT_RENAME_EMPTY(1   8)
-/* (1   9) unused */
+#define DIFF_OPT_NO_IGNORE_SUBMODULE (1   9)
 #define DIFF_OPT_HAS_CHANGES (1  10)
 #define DIFF_OPT_QUICK   (1  11)
 #define DIFF_OPT_NO_INDEX(1  12)
diff --git a/submodule.c b/submodule.c
index 1905d75..9d81712 100644
--- a/submodule.c
+++ b/submodule.c
@@ -301,9 +301,11 @@ void handle_ignore_submodules_arg(struct diff_options 
*diffopt,
DIFF_OPT_CLR(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
DIFF_OPT_CLR(diffopt, IGNORE_DIRTY_SUBMODULES);
 
-   if (!strcmp(arg, all))
+   if (!strcmp(arg, all)) {
+   if (DIFF_OPT_TST(diffopt, NO_IGNORE_SUBMODULE))
+   return;
DIFF_OPT_SET(diffopt, IGNORE_SUBMODULES);
-   else if (!strcmp(arg, untracked))
+   } else if (!strcmp(arg, untracked))
DIFF_OPT_SET(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
else if (!strcmp(arg, dirty))
DIFF_OPT_SET(diffopt, IGNORE_DIRTY_SUBMODULES);
diff --git a/wt-status.c b/wt-status.c
index b4e44ba..34be1cc 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -462,6 +462,9 @@ static void wt_status_collect_changes_index(struct