Code review comment for lp://staging/~jspashett/bzr/587868_args_handling_cant_debug

Martin [gz] wrote:
> Review: Approve
> This basically looks fine to me, but could do with some tests. It's hard to tell just from the diff that, for instance, the py2exe case is still correct. Perhaps Alexander could help you out with that?
>
> Some follow on comments about specific parts of change already mentioned in review:
>
> + # Bug #588277 remove the --profile-imports from the command line arguments
>
> I realise the below code is just trying to match what the code in the bzr script is doing, but a better option would be to change that to not do `sys.argv.remove('--profile-imports')` at all, and add it to the global args list.
>
>
Yes I agree with that. I am not familiar with the global args list
though. My thought was that sys.argv shouldn't be modified, and that
there should be some args list that is passed around instead of
referencing sys.args from within the lower code. That way we can do what
ever is necessary to prepare the arguments list. Unfortunately that's
too many changes as sys.argv is used in many places as you would expect.

> + from bzrlib.errors import InternalBzrError
> + raise InternalBzrError("len(GetCommandLineW(...)) < len(sys.argv) should not be possible")
>
> Just AssertionError would be fine here.
>
> I'd have no problem with this landing as-is though.
>

If you want I can start up another branch try and do what you say Martin
and if Alexander thinks it's a good idea.