Codegen: Ignore missing required keys in help mode #2944
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| # High-level API Help Message | ||
|
|
||
| This decision _does not_ assume code-generation is used. For the case of code-generation see the [Notes](#notes) section. | ||
|
|
||
| ## Problem | ||
|
|
||
| We want to allow to print the help message no matter what errors happened in `kdbOpen` or `kdbGet`. | ||
|
|
||
| ## Constraints | ||
|
|
||
| - `elektraOpen` should not return a broken `Elektra` instance. | ||
| - The help message can only be printed, if `elektraOpen` returns an `Elektra` instance and no `ElektraError`. | ||
|
|
||
| ## Assumptions | ||
|
|
||
| - We assume that the application in question was correctly installed. | ||
| - We assume `gopts` was mounted. | ||
| - We assume the application was called in _help mode_, i.e. with `-h` or `--help`. | ||
| Otherwise printing the help message is not possible, anyway. | ||
|
|
||
| ## Considered Alternatives | ||
|
|
||
| - Ignore all errors (in help mode): | ||
| Not a feasible solution, because there may have been problems when reading the storage file and therefore, | ||
| the help message may be broken or incomplete. | ||
| - Ignore all errors (in help mode), which occurred after the `gopts` plugin ran: | ||
| Complicated to implement (we need to know about plugin order, etc.). | ||
| Not actually necessary (see [Rationale](#rationale)). | ||
|
|
||
| ## Decision | ||
|
|
||
| Ignore missing `require`d keys (in help mode), but fail for every other error. | ||
|
|
||
| ## Rationale | ||
|
|
||
| Required keys **must** be provided by the user/admin and cannot come from another source (Elektra, app developer, etc.). | ||
| Therefore they will be missing until the user makes changes to the KDB. Before that, no other error should occur (we | ||
| assumed a correct installation). If a user runs `app` for the first time and receives an error about a missing required | ||
| key, they will: | ||
|
|
||
| - know what to do and add the key, thereby fixing the problem. | ||
| - try `app -h`/`app --help` to find out more. The help message may or may not contain useful information. If not they may try 3. | ||
| - read some other documentation to find out more. Ideally this leads them to 1. | ||
|
|
||
| In any case after this the user definitely know how to interact with the KDB. Since we assumed that there won't be any | ||
| errors before the KDB was changed, we can assume that the user caused other errors by changing the KDB. | ||
|
|
||
| ## Notes | ||
|
|
||
| If code-generation is used, the situation is a little different. If the parameter `embedHelpFallback` is set to `1`, a | ||
| fallback help message will be created from the specification originally passed to the code-generator and embedded into | ||
| the application. The parameter also changes, how help mode is detected and ultimately allows the help message function | ||
| (`printHelpMessage` by default) to always print a help message. Although it may not reflect changes the user made to the | ||
| specification. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,7 +38,7 @@ If you specifically want to use it with the High-Level API take a look at [this | |
|
|
||
| We also created a new CMake function that will be available, if you include Elektra via CMake's | ||
| `find_package`. The function is called `elektra_kdb_gen` and can be used to tell CMake about files | ||
| that are generated via `kdb gen`. _(Klemens Böswirth)_ | ||
|
|
||
| ### <<HIGHLIGHT2>> | ||
|
|
||
|
|
@@ -80,6 +80,10 @@ The following section lists news about the [modules](https://www.libelektra.org/ | |
|
|
||
| - We now treat relative paths as relative to `KDB_DB_SPEC` instead of the current working directory. _(Klemens Böswirth)_ | ||
|
|
||
| ### Spec | ||
|
|
||
| - There is now the config key `missing/log` that allows logging of all missing `require`d keys. _(Klemens Böswirth)_ | ||
|
There was a problem hiding this comment. Shouldn't we fail if a required key is missing? Why do we need the log as this should already be part of the error message? There was a problem hiding this comment.
There was a problem hiding this comment.
This is one of spec's anti-features. Thank you for documenting it but I would be in favor of removing these anti-features. If validation fails, the plugin should return an error.
Ok. But wouldn't it be better to directly ask for a meta-key which says that we are in the help mode? We want the help to be printed no matter what errors happened in There was a problem hiding this comment.
IMO the option to switch to warnings instead might be useful.
I agree, however, that ignoring all problems by default is bad. I think it was done, so that problems can still be fixed via
We are doing that.
I thought about that, but I am not sure this is possible. Ignoring all errors from Required keys must be provided by the user/admin and cannot come from another source (Elektra, app developer, etc.). Therefore they will be missing until the user makes changes to the KDB. Before that, no other error should occur (°). If a user runs
In any case after this the user definitely know how to interact with the KDB. Since we assumed that there won't be any errors before the KDB was changed, we can assume that the user caused other errors by changing the KDB. Of course it would be nice to always show the help message, but since it depends on the specification being readable, that is somewhat impossible. One way around that (at least in applications using codegen) is to generate a fallback help message that is used, if we couldn't read the spec. This fallback help message would be based on the spec provided to the code generator. It would therefore not reflect any changes to the spec (e.g. (°) assuming a correct installation. But broken installations cannot be ignored anyway, since they may interfere with the help message that is generated from the spec. There was a problem hiding this comment.
This would also be possible if the first error is as error and the others as warning.
We usually only add warnings during kdbGet() and errors only in kdbSet(). I would also expect that behavior in the spec plugin. We should document this behavior in a decision.
Very nice write-up. Can you make a decision of what you wrote here? There was a problem hiding this comment.
There are many cases, where For the
Will do. There was a problem hiding this comment. |
||
|
|
||
| ## Libraries | ||
|
|
||
| The text below summarizes updates to the [C (and C++)-based libraries](https://www.libelektra.org/libraries/readme) of Elektra. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,9 @@ libelektra_0.8 { | |
| elektraKeyGetRelativeName; | ||
| elektraKsFilter; | ||
| elektraResolveReference; | ||
| }; | ||
|
|
||
| libelektra_0.9 { | ||
|
There was a problem hiding this comment. It should have been part of #2916. The functions were not part of any 0.8.x release, so they should not be versioned as If we won't have another 0.9.x release and go straight to 1.0.0, they should actually be in There was a problem hiding this comment. We will definitely have a 0.9 release before 1.0. |
||
| ## ToString; | ||
| elektraBooleanToString; | ||
| elektraCharToString; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,6 +34,35 @@ static void cleanupEnvp (char ** envp); | |
| #error "No implementation available" | ||
| #endif | ||
|
|
||
| /** | ||
| * Detects whether we are in help mode or not. | ||
| * DOES NOT set 'proc/elektra/gopts/help' for use with elektraGetOptsHelpMessage(). | ||
| * | ||
| * @retval 1 if the -h or --help is part of argv | ||
| * @retval 0 otherwise | ||
| * @retval -1 on error (could not load argv) | ||
| */ | ||
| int elektraGOptsIsHelpMode (void) | ||
| { | ||
| char ** argv = NULL; | ||
| int argc = loadArgs (&argv); | ||
|
|
||
| if (argv == NULL) | ||
| { | ||
| return -1; | ||
| } | ||
|
|
||
| for (int i = 0; i < argc; ++i) | ||
| { | ||
| if (strcmp (argv[i], "-h") == 0 || strcmp (argv[i], "--help") == 0) | ||
|
There was a problem hiding this comment. I do not like that this is hardcoded here. Furthermore this is not properly parsed, e.g. There was a problem hiding this comment.
Using This function is not actually intended to accurately detect help mode (for that one should still check for the presence of Apart from
See above #2944 (comment). There was a problem hiding this comment.
Because of how the cmdline options in Elektra are implemented every option needs to have a short option.
It makes a lot of sense to not have a --help output and a separated man page. E.g. git also does it this way.
I would prefer if we only hardcode |
||
| { | ||
| return 1; | ||
| } | ||
| } | ||
|
|
||
| return 0; | ||
| } | ||
|
|
||
|
|
||
| int elektraGOptsGet (Plugin * handle ELEKTRA_UNUSED, KeySet * returned, Key * parentKey) | ||
| { | ||
|
|
@@ -43,6 +72,7 @@ int elektraGOptsGet (Plugin * handle ELEKTRA_UNUSED, KeySet * returned, Key * pa | |
| ksNew (30, keyNew ("system/elektra/modules/gopts", KEY_VALUE, "gopts plugin waits for your orders", KEY_END), | ||
| keyNew ("system/elektra/modules/gopts/exports", KEY_END), | ||
| keyNew ("system/elektra/modules/gopts/exports/get", KEY_FUNC, elektraGOptsGet, KEY_END), | ||
| keyNew ("system/elektra/modules/gopts/exports/ishelpmode", KEY_FUNC, elektraGOptsIsHelpMode, KEY_END), | ||
|
There was a problem hiding this comment. isinhelpmode or simply helpmode? There was a problem hiding this comment. Since you already merged this PR: Do you still want the change? There was a problem hiding this comment. It was only a question. If you like ishelpmode best, we can keep it. |
||
| #include ELEKTRA_README | ||
| keyNew ("system/elektra/modules/gopts/infos/version", KEY_VALUE, PLUGINVERSION, KEY_END), KS_END); | ||
| ksAppend (returned, contract); | ||
|
|
||
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.
Is this already default?
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.
goptsis not mounted by default no. However, the file generated by thehighleveltemplate, include akdbEnsurecontract, that mounts it, if it is missing.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.
Please copy this sentence into the decision.