On Mon, Apr 24, 2017 at 3:11 PM, Ramsay Jones
<ramsay@xxxxxxxxxxxxxxxxxxxx> wrote:
>
>
> On 24/04/17 21:03, Stefan Beller wrote:
> [snip]
>
>> +
>> + argv_array_pushf(&cp.env_array, "name=%s", sub->name);
>> + argv_array_pushf(&cp.env_array, "path=%s", displaypath);
>> + argv_array_pushf(&cp.env_array, "sm_path=%s", displaypath);
>>
>> You mention keeping 'sm_path' in the notes after the commit message. I would
>> add that part to the commit message, to explain why we have multiple variables
>> that have the same value. Maybe even a comment in the code:
>>
>> /* Keep sm_path for historic reasons, see tests in 091a6eb0fee. */
>> .. sm_path ..
>
> Hmm, you need to be a bit careful with putting 'path' in the
> environment (if you then export it to sub-processes) on windows
> (cygwin, MinGW, GfW). See commit 64394e3ae9. I would have liked
> to remove $path altogether from the 'submodule-foreach api' in
> that commit, but users and their scripts were already using it
> (so I couldn't just drop it, without some deprecation period).
> So long as whatever was being 'eval'-ed in the script didn't
> export $path, ...
>
Oh, I misread the comment
# we make $path available to scripts ...
path=$sm_path
as it was such a casual friendly thing to say in that context.
So the *real* historic baggage is
argv_array_pushf(&cp.env_array, "path=%s", displaypath);
whereas
argv_array_pushf(&cp.env_array, "sm_path=%s", displaypath);
is considered the correct way to go.
Thanks,
Stefan