git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
To: Sangeeta NB <sangunb09@gmail.com>
Cc: git@vger.kernel.org, phillip.wood@dunelm.org.uk
Subject: Re: [Outreachy] Introduction
Date: Mon, 12 Oct 2020 16:52:14 +0530	[thread overview]
Message-ID: <943d84e6-98fb-29e3-086a-3763c9e73ef8@gmail.com> (raw)
In-Reply-To: <CAHjREB6j6BqZ49wX5uqEOiysTAm8Oo7N=EFpcoovWKkBghBjxQ@mail.gmail.com>

Hi Sangeeta,

On 11/10/20 5:00 pm, Sangeeta NB wrote:
> 
>> As I understand it if a submodule contains any untracked files (i.e. a
>> file that has not been added with `git add` and is not ignored by any
>> .gitignore or .git/info/exclude entries) then running `git diff` in the
>> superproject will report that the submodule is dirty - there will be a
>> line something like "+Subproject commit abcdef-dirty". However if we run
>> `git describe --dirty` in the submodule directory then it will not
>> append "-dirty" to it's output unless there are changes to tracked files.
> 
> On running `git diff HEAD --ignore-submodules=untracked` the submodule
> wasn't reported as dirty.
>

Right.

> I guess this is what we are expecting. So should I make it the default
> behavior for diff?
> 

Yeah. Changing the default behaviour of 'diff' looks like one of the
options that Junio mentions in [1].

> A fix for making this as the default behaviour can be:
> 
> --- a/diff.c
> +++ b/diff.c
> @@ -422,6 +422,7 @@ int git_diff_ui_config(const char *var, const char
> *value, void *cb)
>          if (git_color_config(var, value, cb) < 0)
>                  return -1;
> 
> +       handle_ignore_submodules_arg(&default_diff_options, "untracked");
>          return git_diff_basic_config(var, value, cb);
>   }
> 

I'm not sure about whether 'git_diff_ui_config' is the right
place to do this, though. I'm also not sure about what the right
approach would be, off-hand. But I believe the Junio's e-mail I
reference might be of help in pointing you in the right direction, in
general.

> But this would also involve a lot of changes in the way tests are
> written as 12 out of 19 tests in t4027-diff-submodule.sh failed after
> adding this patch. I am working on any other workaround for this. Let
> me know whether I am on right path or not. Also any pointers on how to
> proceed would be helpful. Thanks!
> 

Some test failures are likely to happens as a consequence of the change
given that we would be changing default behaviour. So, adjusting the
tests appropriately would indeed be necessary. We would've to be careful
in evaluating the failures so that we don't break other _valid_ use
cases as a side-effect.

[ References ]

[1]: https://lore.kernel.org/git/xmqq1rixi4cb.fsf@gitster.c.googlers.com/

--
Sivaraam

  parent reply	other threads:[~2020-10-12 11:22 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-07 20:10 [Outreachy] Introduction Sangeeta NB
2020-10-08  9:07 ` Phillip Wood
2020-10-09  7:41   ` Sangeeta NB
2020-10-09 18:29     ` Phillip Wood
2020-10-11 11:30       ` Sangeeta NB
2020-10-12 10:18         ` Phillip Wood
2020-10-12 11:22         ` Kaartic Sivaraam [this message]
2020-10-12 15:57         ` Junio C Hamano
2020-10-14 15:52           ` Sangeeta NB
2020-10-15  9:23             ` Phillip Wood
2020-10-15  9:26               ` [PATCH] fixup! diff: do not show submodule with untracked files as "-dirty" Phillip Wood
2020-10-15 10:18               ` [Outreachy] Introduction Sangeeta NB
2020-10-15 13:39                 ` Phillip Wood
2020-10-15 13:57                   ` Sangeeta NB
2020-10-15 14:45                     ` Phillip Wood
2020-10-16  5:27                       ` Sangeeta NB
2020-10-16 13:26                         ` Phillip Wood
  -- strict thread matches above, loose matches on Subject: below --
2020-10-10 11:48 Charvi Mendiratta
2020-10-11  8:09 ` Christian Couder
     [not found]   ` <CAPSFM5cXN57z56Cvq-NX1H4raS7d8=qXEFDQqpypJfoYzbxcyA@mail.gmail.com>
2020-10-15 18:56     ` Charvi Mendiratta
2020-10-15 19:16       ` Junio C Hamano
2020-10-17  8:09         ` Charvi Mendiratta
2020-10-16  8:28 Zodwa Phakathi
2020-10-16  8:46 ` Christian Couder
     [not found]   ` <CAGdqGXrLN2W_CgqfmfkCSu_hmZ9Ze8A1N9n08bgPRPApSMraSQ@mail.gmail.com>
2020-10-16 10:02     ` Christian Couder
2020-10-16 22:09 Joey S
2020-10-16 23:08 ` Jonathan Nieder
2020-10-17  0:42   ` Joey S

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=943d84e6-98fb-29e3-086a-3763c9e73ef8@gmail.com \
    --to=kaartic.sivaraam@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=sangunb09@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).