-
Notifications
You must be signed in to change notification settings - Fork 3
CLI reorganization: desired state of CLI help #74
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
Conversation
Do Not Merge: these are changes to a generated file, instead change the actual agama CLI
|
The "command overview" section is not updated accordingly. That's confusing. And the config commands are split. First you find the ones that has always been "config", then some others (like "probe" or "install"), and then the ones that used to be "profile" but are "config" now. Hard to follow. |
|
|
||
| ###### **Arguments:** | ||
|
|
||
| * `<URL_OR_PATH>` — JSON file, URL or path or `-` for standard input |
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.
Why do you need to specify -? I think that not specifying an URL_OR_PATH should be enough.
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.
BTW I was unsure so I asked Google AI for help.
Gemini 2.0 Flash: started off by getting cat wrong (saying that cat needed an explicit argument)
Gemini Deep research 2.5 Pro: its report convinced me that requiring an explicit - is too strict
|
|
||
| Kinds: | ||
| - JSON | ||
| - Jsonnet, injecting the hardware information from D-Bus (WTF D-Bus??) |
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 wrong. It does not use D-Bus at all.
| ## `agama profile import` | ||
|
|
||
| Process autoinstallation profile and loads it into agama | ||
| For an example of Jsonnet-based profile, see https://github.com/openSUSE/agama/blob/master/rust/agama-lib/share/examples/profile.jsonnet |
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.
It should point to some profile at agama-project.github.io which should be a better place for user's documentation.
|
|
||
| * `<URL>` — Profile's URL. Supports the same schemas as the "download" command plus AutoYaST specific ones. Supported files are json, jsonnet, sh for Agama profiles and ERB, XML, and rules/classes directories for AutoYaST support | ||
|
|
||
| * `<URL_OR_PATH>` — file, URL or path or `-` for standard input |
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.
Reading from the standard input could be useful to translate from Jsonnet or XML.
| The purpose of this command is to download files using AutoYaST supported schemas (e.g. device:// or relurl://). It can be used to download additional scripts, configuration files and so on. You can use it for downloading Agama autoinstallation profiles. However, unless you need additional processing, the "agama profile import" is recommended. If you want to convert an AutoYaST profile, use "agama profile autoyast". | ||
|
|
||
| **Usage:** `agama download <URL> <DESTINATION>` | ||
| **Usage:** `agama download [OPTIONS] <URL>` |
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.
This command prints some information about which devices is trying. In the future, it could even display some progress information. For that reason, we might want to add a flag to disable/enable such a behavior (-q or -v, I am not sure what is better).
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.
curl is similar in displaying progress via stderr, maybe we want to model that behavior: https://www.man7.org/linux/man-pages/man1/curl.1.html#PROGRESS_METER
| - AutoYaST profile, including ERB and rules/classes | ||
| Locations: | ||
| - path | ||
| - URL (inlcuding AutoYaST specific schemes) |
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.
Typo: inlcuding
| * `<URL_OR_PATH>` — JSON file, URL or path or `-` for standard input | ||
|
|
||
|
|
||
| ## `agama config generate` |
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 don't fully understand how this command works.
Does it take any AutoYaST/Agama profile as input and generates the Agama config as output?
Does it export the current config into all those formats (I don't think so, but..)?
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.
It turns an AutoYaST or an Agama Jsonnet/JSON file and generates de configuration output. A few notes:
- If given a URL, it downloads the file. Bear in mind that in AutoYaST, downloading the profile could imply more than one file (e.g., rules/classes).
- For a JSON file, it just downloads the file and solves the relative URLs.
- Reading from the stdin prevents downloading any file. It might be useful to convert an single-file AutoYaST profile or a Jsonnet one.
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 understood it after writing my comment. I guess that "AutoAgama" will pipe a "config generate" to a "config load".
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.
So that current internal conversion becomes somehow more explicit and exposed.
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.
Yes, that's the point, to make the conversion more explicit. However, we cannot do agama config generate URL | agama config load because we might need to validate the result in the middle. But that's a different topic.
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.
However, we cannot do agama config generate URL | agama config load because we might need to validate the result in the middle. But that's a different topic.
It is quite relevant. I am leaning toward making validation automatic:
- automatic and mandatory on input (
load,edit) - automatic but optional (
--no-validate) for output (generate,show). Remember how AY would export profiles that would not validate? Not Agama. - explicit by
validate(which in light of the above is justconfig blackholebut with an automatic validation part)
This way, agama config generate URL | agama config load performs validation twice, but I think it is a small price to pay for the resulting robustness for users and developers.
@ancorgs Yes, sorry, that's intentional so that the diff makes it clear what is added and what is removed, at the expense of the resulting document. Which will be fixed when it is rendered by clap + xtask. |
|
Implemented in agama-project/agama#2347 |
As a summary of agama-project/agama#2289, this is the model how the CLI help should look like. Not to be merged here, but implemented in the main Agama repo.