William Deegan

Results 322 comments of William Deegan

I refactored your fix, I'm not a big fan of x if xyz else abc form. This way we're clearly calling out the invalid None value.

@jcbrill as usual your suggested fix and analysis are well thought out! If I'm understanding correctly though, there's no good way to test this unless we fired up a VM...

Would it be possible to prevent this by only making changes in OverrideEnvironment()? That way there'd be zero performance hit for non OverrideEnvironment() usage, and potentially none for it's parent...

> Not sure I'm seeing an issue? The track-deletes behavior _is_ only in `OverrideEnvironment`. Anywhere you've replaced self._dict with self['xyz]. Is a change to regular environment and potentially a performance...

> I did most of those as a separate commit so it's easy to roll back. But... for `self['xyz']`, if it's a regular env, then it uses `SubstitutionEnvironment`'s `__getitem__` and...

> Updated - please glance at the edited summary at the top, maybe more clear now. Had to force-push due to branch merging done since the original push getting things...

Looks like test: `test/AddOption/args-and-targets.py` is failing? Can you take a look and resolve?

> This ended up sitting for a long time. @bdbaddog - in terms of at least fixing #3798, would it be more palatable to pick just that part of this...

Closing. A lot of this functionality addressed by @mwichmann . If there's more to be done, he'll add additional PRS.

Not sure why you believe this change is needed? local_only, means only the local addoptions args are in the help.. what's the concern here?