Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a "Prepend" extension for the ProcessArgumentBuilder #1281

Closed
antwilliams opened this issue Oct 13, 2016 · 5 comments
Closed

Add a "Prepend" extension for the ProcessArgumentBuilder #1281

antwilliams opened this issue Oct 13, 2016 · 5 comments
Labels
Milestone

Comments

@antwilliams
Copy link
Contributor

It would be useful to add an Insert method for the ProcessArgumentBuilder to complement the Append method.

This would allow additional arguments to be specified for tools that use the final argument as the target.

An example (and my motivation) of this is the SignTool command, I need to add an additional flag that isn't supported by the current tool implementation. If I use the ArgumentCustomization property the new flag is added to the end of the arguments and then SignTool interprets this as a filename to try and sign.

The flag I require could be added to the SignTool implementation but that would only resolve this specific use case, I would imagine that there are many tools that use the final argument as the input filename and this modification would enable those scenarios as well.

I am willing to submit a PR for this if there is interest in the improvement.

@antwilliams antwilliams changed the title New Feature: Add an "Insert" extension for the ProcessArgumentBuilder Add an "Insert" extension for the ProcessArgumentBuilder Oct 13, 2016
@gep13 gep13 added the Feature label Oct 13, 2016
@gep13
Copy link
Member

gep13 commented Oct 13, 2016

In principal, I don't have a problem with this. The only suggestion would be the rather Insert which would suggest inserting at a particular location, the method might be called Prepend instead?

@gep13
Copy link
Member

gep13 commented Oct 13, 2016

And yes, perhaps raise another issue for adding the Target property to the SignTool implementation.

@agc93
Copy link
Member

agc93 commented Oct 13, 2016

As @gep13 mentioned, Prepend seems more appropriate since most Insert methods (even including StringBuilder for example) seem to be for inserting into a specified location.

@antwilliams antwilliams changed the title Add an "Insert" extension for the ProcessArgumentBuilder Add a "Prepend" extension for the ProcessArgumentBuilder Oct 13, 2016
@antwilliams
Copy link
Contributor Author

Makes sense to me, I had been thinking about an Insert method that would allow a position to be specified but on further consideration this wouldn't make much sense.
The user couldn't guarantee what other arguments were being added by the tool implementation and wouldn't be able to guarantee the final position.

I'll start on a PR!

@gep13
Copy link
Member

gep13 commented Oct 19, 2016

@antwilliams thanks again for your contribution!

@gep13 gep13 closed this as completed Oct 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants