Page MenuHomePhabricator

[lit] Print internal env commands

Authored by jdenny on Aug 1 2019, 5:25 PM.



Without this patch, the internal env command removes env and its
args from the command line while parsing it. This patch modifies a
copy instead so that the original command line is printed.

Diff Detail


Event Timeline

jdenny created this revision.Aug 1 2019, 5:25 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 1 2019, 5:25 PM
Herald added a subscriber: delcypher. · View Herald Transcript
stella.stamenova accepted this revision.Aug 2 2019, 9:48 AM
This revision is now accepted and ready to land.Aug 2 2019, 9:48 AM
rnk added a comment.Aug 2 2019, 11:33 AM

Suggestion, but this looks good too.

881 ↗(On Diff #212939)

I think it would be clearer to make updateEnv return the new command to run after env instead of modifying cmd.args. We copy cmd.args on line 903 currently anyway.

915 ↗(On Diff #212939)

If you take my suggestion, this needs to use args not j.args, since it will refer to "env" and not the real exe for env-prefixed commands.

jdenny updated this revision to Diff 213115.Aug 2 2019, 1:15 PM
jdenny edited the summary of this revision. (Show Details)

Apply rnk's suggestion, and adjust updateEnv to make it clear it only operates on the arguments.

@rnk, @stella.stamenova : thanks.

I'll wait a bit to push in case there are further comments.

jdenny marked 2 inline comments as done.Aug 2 2019, 1:16 PM
rnk accepted this revision.Aug 2 2019, 2:09 PM

lgtm, thanks!

This revision was automatically updated to reflect the committed changes.