[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form

Wed Aug 11 13:30:56 PDT 2021

jingham added a comment.

In D107869#2940016 <https://reviews.llvm.org/D107869#2940016>, @OmarEmaraDev wrote:

> @jingham I wasn't arguing that we should remove those environment variables, on the contrary. Greg was suggesting that we populate the environment field with the target environment instead of implicitly adding them to the environment of the launch info. The problem with that is that there is a large number of environments that gets added (15 in my case) as shown in the following screenshot. What I was saying in my comment above is that I don't think we should show those to the user, we should transparently add them instead and let the user add additional variables as needed. We can then look into adding other settings to exclude them if necessary.
>
> F18486746: 20210811-210121.png <https://reviews.llvm.org/F18486746>

Thanks for the clarification.

I still think people will want to see the actual environment that is being provided to the inferior.  They don't want to have to do our merging algorithm in their heads.

For instance, if lldb had an environment variable that wasn't appropriate for the inferior, I would want to see that in the Environment View, so I could turn it off.  If that variable is passed silently to the inferior, and not shown anywhere in the launch UI, figuring out that I needed to not inherit that one would be more difficult.

However, if you feel like the really useful affordance here is for "User Provided Environment Variables", that's also a valid choice, but then you should name it appropriately, so as not to cause confusion.  If there's a window listing in the launch parameters panel called "Environment Variables" it should be the environment variables the process will launch with, since that's the plain meaning of the title.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107869/new/

https://reviews.llvm.org/D107869