-
Notifications
You must be signed in to change notification settings - Fork 258
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 spec of dotnet nuget config command #12172
Merged
Merged
Changes from 2 commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
e8fdc34
work
heng-liu 006cb0f
fix
heng-liu 335e6ae
address feedback
heng-liu 204ba1a
fix
heng-liu 5dff9f6
address feedback
heng-liu abc436d
address feedback
heng-liu ffd5e71
address comments
heng-liu a25ea5d
address feedback
heng-liu 9da2a9f
address comments & add more examples
heng-liu a1f68cf
add verbosity detailed
heng-liu 3fe2fe3
address feedback, list merged settings by default
heng-liu 1bcd070
add set and unset
heng-liu c261b0b
fix and add a question
heng-liu 54133da
add consideration
heng-liu 91e47c0
fix format
heng-liu a64746c
work
heng-liu dfae3bd
address comments
heng-liu 209f3f0
edit the open questions
heng-liu efb1a2e
fix
heng-liu b56944d
fix format
heng-liu 4150a2b
fix
heng-liu b74b829
fix option working-dir
heng-liu b679074
address feedback
heng-liu 3b4ef11
Reformat the output of dotnet nuget config all
heng-liu 46aec6c
address feedback
heng-liu 0836860
address feedback
heng-liu e4b6ae5
change WORKING_DIRECTORY from option to argument
heng-liu 160cda0
address feedbacks
heng-liu 779cfc8
fix a typo
heng-liu 45f081f
address feedback
heng-liu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
# NuGet configuration CLI for dotnet.exe | ||
|
||
- Author: [Heng Liu](https://github.com/heng-liu) | ||
- GitHub Issue [8420](https://github.com/NuGet/Home/issues/8420) | ||
|
||
## Problem Background | ||
|
||
Currently, there is no NuGet configuration CLI for dotnet.exe. It's inconvenient for NuGet users to know about the NuGet configuration file locations and figure out where is the merged configuration coming from. | ||
|
||
## Who are the customers | ||
|
||
This feature is for dotnet.exe users. | ||
|
||
## Goals | ||
Design and implement `dotnet nuget config` command. | ||
|
||
## Non-Goals | ||
Design and implement `dotnet nuget config` command with commands other than `list`, E.g. add/update/delete | ||
|
||
## Solution | ||
The following command will be implemented in the `dotnet.exe` CLI. | ||
|
||
### `dotnet nuget config` | ||
|
||
#### Commands | ||
If no command is specified, the command will default to list. | ||
heng-liu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
- List | ||
|
||
Lists all the NuGet configuration file locations. This command will include all the NuGet configuration file that will be applied, when invoking NuGet command from the current working directory path. The listed NuGet configuration files are in priority order. So the order of loading those configurations is reversed, that is, loading order is from the bottom to the top. So the configuration on the top will apply. | ||
You may refer to [How settings are applied](https://learn.microsoft.com/en-us/nuget/consume-packages/configuring-nuget-behavior#how-settings-are-applied) for more details. | ||
|
||
#### Options | ||
- -?|-h|--help | ||
|
||
Prints out a description of how to use the command. | ||
|
||
#### Examples | ||
|
||
- List all the NuGet configuration file that will be applied. | ||
|
||
``` | ||
dotnet nuget config list | ||
|
||
c:\repos\Solution\Project\NuGet.Config | ||
heng-liu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
c:\repos\Solution\NuGet.Config | ||
C:\Users\username\AppData\Roaming\NuGet\NuGet.Config | ||
C:\Program Files (x86)\NuGet\Config\Microsoft.VisualStudio.FallbackLocation.config | ||
C:\Program Files (x86)\NuGet\Config\Microsoft.VisualStudio.Offline.config | ||
``` | ||
|
||
## Future Work | ||
The `dotnet nuget config list` is a community ask. We will consider adding more commands, like add/update/delete, in the future. | ||
|
||
## Open Questions | ||
1. Shall we add `working directory` as an option? So that people could check different NuGet configuration lists without changing current directory. | ||
erdembayar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
2. Do we need to add this into NuGet.exe CLI? | ||
|
||
## Considerations | ||
1. NuGet.exe [config command](https://learn.microsoft.com/en-us/nuget/reference/cli-reference/cli-ref-config) is implemented. But there is no `list` command. And the behavior is confusing (the `set` command will set the property which appears last when loading, so sometimes it's not updating the closest NuGet configuration file). Do we want to implement those subcommand(e.g.`set`) in the future in dotnet.exe differently? | ||
heng-liu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really wish GitHub had threaded/resolvable conversations for comments without a specific file/line number 😔
I'd like to know what the design is for error scenarios, specifically when one or more of the XML files contains syntax errors (from an implementation point of view, this means that
Settings.Load*
apis will fail, so we can't useloadedConfig.GetConfigFilePaths()
).Is
dotnet nuget config list
going to report an error and fail to list all the files? Will it at least tell us the filename of the XML that couldn't be parsed? Or will all the XML files be listed despite the parsing failure? Will it tell us which file(s) contain parsing errors?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good question!
I tested with NuGet.exe and dotnet.exe, and both of them are able to tell us the filename of the XML that couldn't be parsed, as below:
So I think the
dotnet nuget config list
is able to do this as well.If we'd like to list all config files despite the parsing failure, we'd consider if it worth doing.
Since running all nuget commands will encounter this invalid XML error, users need to fix this invalid XML before running any NuGet commands. So this seems to be a prerequisite and there is no harm if user needs to do this first before knowing the list of config files.
In addition, this error message clearly points out the file path and location, so it should be easy for users to correct the invalid XML.
The only scenario needs to be considered is, with the invalid XML error, user should able to specifying a specific correct config file to run nuget commands. And if we list all config file locations despite the parsing failure, it might help people to find the path easier. But it seems "listing all config file locations despite the parsing failure" adds very limited value.
Those are my preliminary thoughts. Please let me know if you have any other thoughts :)
The other error scenario I'd like to talk about is, the specified working directory doesn't exist. Shall we go ahead listing user-wide and machine-wide config files? Or show a warning saying the working directory doesn't exist? Or both?