On Tue, Aug 16, 2016 at 2:14 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Jacob Keller <jacob.kel...@gmail.com> writes:
>>>> +
>>>> +     if (dirty_submodule & DIRTY_SUBMODULE_MODIFIED) {
>>>> +             /*
>>>> +              * If the submodule has modified contents we want to diff
>>>> +              * against the work tree, so don't add a second parameter.
>>>> +              * This is essentially equivalent of using diff-index 
>>>> instead.
>>>> +              * Note that we can't (easily) show the diff of any untracked
>>>> +              * files.
>>>> +              */
>>> ...
>>> I am debating myself if this is a good thing to do, though.  The
>>> submodule is a separate project for a reason, and there is a reason
>>> why the changes haven't been committed.  When asking "what's different
>>> between these two superproject states?", should the user really see
>>> these uncommitted changes?
>> Well, the previous submodule code for "log" prints out "submodule has
>> untracked content" and "submodule has modified content" so I figured
>> the diff might want to show that as a diff too? Or maybe we just stick
>> with the messages and only show the difference of what's actually been
>> committed.. I think that's probably ok too.
> I do not have a strong opinion.  We only see DIRTY when we are
> looking at the working tree at the top-level diff (i.e. "git diff
> HEAD~ HEAD" at the top-level, or "git diff --cached" will not show
> DIRTY_SUBMODULE_MODIFIED when a submodule has local modifications in
> its working tree), so in that sense, it probably makes sense to show
> the more detailed picture of the working tree like your patch does.
> After all, choosing to use --submodule=diff is a strong sign that
> the user who says top-level "git diff [<tree>]" wants to see the
> details of her work-in-progress.
> Do you need to handle "git diff -R [<tree>]" at the top-level a bit
> differently, by the way?  If this function gets the full diff_options
> that is driving the top-level diff, the DIFF_OPT_REVERSE_DIFF bit
> would tell you that.

Probably. Unfortunately what I really need is to be able to convert
(almost) all diff options from the diff_options structure back into
command line flags. This is the primary reason I would prefer to not
use a sub-command, but I'm not really sure what's the best way to
actually DO that in a safe way.

The sub command brings nice separation between the superproject and
its submodules... but it has problem because we can't use C calls
directly, so I can't pass the options along myself.

Thoughts on that? Or should we just limit ourselves to only some
options get propagated to the submodule?

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

Reply via email to