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

Command.Create should not transitively depend on hard coded paths #4592

Closed
cdmihai opened this issue Dec 16, 2015 · 3 comments
Closed

Command.Create should not transitively depend on hard coded paths #4592

cdmihai opened this issue Dec 16, 2015 · 3 comments
Assignees
Milestone

Comments

@cdmihai
Copy link
Contributor

cdmihai commented Dec 16, 2015

The resolution of commands depend on Directory.GetCurrentDirectory() and AppContext.BaseDirectory(). This introduces an implicit dependency between command resolution and the process that requests it (basedirectory) and the root process that spawned everything (getcurrentdirectory).

Instead, these paths should be variables resolved / injected at runtime.

This change enables:

  • decoupling command resolution from process identity
  • increased testability of the command resolution algorithm (can setup a fake environment and assert for resolution outcome)
  • calling verbs in-proc and out-of-proc
@TheRealPiotrP
Copy link
Contributor

@cdmihai do you have a proposed design? I'd like to see how it would impact UX for the product.

@cdmihai
Copy link
Contributor Author

cdmihai commented Jan 12, 2016

I don't think this would impact the user. The change suggests replacing the hard coded paths from below with parameters, that would probably get sent to TryResolveCommandSpec and then trickled down (or maybe transform CommandResolver into a non static class whose instances get constructed with them)

That would make the CommandResolver quite configurable (good for writing tests that send funky paths, good for build to ensure the command is resolved as it would have been by Compile, etc)

wli3 referenced this issue in wli3/cli Jul 14, 2017
@msftgits msftgits transferred this issue from dotnet/cli Jan 31, 2020
@msftgits msftgits added this to the Backlog milestone Jan 31, 2020
rainersigwald pushed a commit that referenced this issue Jul 20, 2020
…4f-4d3a-8a02-00e49a44a1cc

[release/3.1.4xx] Update dependencies from Microsoft/msbuild
@baronfel
Copy link
Member

This is no longer applicable to the parser now that we're on System.CommandLine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants