On Fri, Dec 9, 2016 at 1:55 AM, Stefan Beller <sbel...@google.com> wrote:
> On Thu, Dec 8, 2016 at 2:09 AM, Duy Nguyen <pclo...@gmail.com> wrote:
>> On Thu, Dec 8, 2016 at 8:46 AM, Stefan Beller <sbel...@google.com> wrote:
>>>
>>>         worktree = xcalloc(1, sizeof(*worktree));
>>>         worktree->path = strbuf_detach(&worktree_path, NULL);
>>> @@ -101,7 +101,8 @@ static struct worktree *get_main_worktree(void)
>>
>> All the good stuff is outside context lines again.. Somewhere between
>> here we call add_head_info() which calls resolve_ref_unsafe(), which
>> always uses data from current repo, not the submodule we want it to
>> look at.
>
> Unrelated side question: What would you think of "variable context line
> configuration" ? e.g. you could configure it to include anything from
> up that line
> that is currently shown after the @@ which is the function signature line.

Hmm.. no idea. I once dreamt of writing "Diff-Options: -U10" in the
commit message and let git-log and everybody use that option
automatically, though. I guess it's unrelated to your question :D

> As to the add_head_info/resolve_ref_unsafe what impact does that have?
> It produces a wrong head info but AFAICT it will never die(), such that for 
> the
> purposes of this series (which only wants to know if a submodule uses the
> worktree feature) it should be fine.
>
> It is highly misleading though for others to build upon this.
> So maybe I'll only add the functionality internally in worktree.c
> and document why the values are wrong, and only expose the
> "int submodule_uses_worktrees(const char *path)" ?

Yeah for submodule use then it should be ok. But people may start
using it for something else, not realizing that it does not work as
expected.
-- 
Duy

Reply via email to