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

How to set a negative value with kdb set #2468

Closed
ghost opened this issue Mar 8, 2019 · 12 comments
Closed

How to set a negative value with kdb set #2468

ghost opened this issue Mar 8, 2019 · 12 comments

Comments

@ghost
Copy link

ghost commented Mar 8, 2019

How can I set a negative number in kdb?

kdb set '/my/number' '-3'                                                   
#> kdb: invalid option -- '3'
#> Sorry, I could not process the given options (see errors above)
#> 
#> Usage: kdb set <name> [<value>]
#> 
#> Set the value of an individual key.
#> If no value is given, it will be set to a null-value
#> To get an empty value you need to quote like "" (depending on shell)
#> To set a negative value you need to use '--' to stop option processing.
#> (e.g. 'kdb set -- /tests/neg -3')
#> 
#> -H --help                Show the man page.
#> -p --profile <name>      Use a different profile for kdb configuration.
#> -v --verbose             Explain what is happening.
#> -q --quiet               Only print error messages.
#> -V --version             Print version info.
#> -N --namespace <ns>      Specify the namespace to use for cascading keys.
#> -C --color <when>       Print never/auto(default)/always colored output.

" does not help either

@sanssecours
Copy link
Member

sanssecours commented Mar 8, 2019

I think you should take a closer look at the error message 😊:

#> To set a negative value you need to use '--' to stop option processing.

, which tells you that something like

kdb set /user/tests/negative -- -1
#> Using name user/user/tests/negative
#> Set string to "-1"

should work.

@ghost
Copy link
Author

ghost commented Mar 8, 2019

Thank you, under this extremely verbose output I did not read that sentence.

@ghost ghost closed this as completed Mar 8, 2019
@markus2330 markus2330 added bug and removed question labels Mar 9, 2019
@markus2330
Copy link
Contributor

The text could be improved:

"see errors above" is misleading, "see lines above" might be better?

"If no value is given, it will be set to a null-value" and the following sentence does not have a dot at the end.

@markus2330 markus2330 reopened this Mar 9, 2019
@markus2330 markus2330 assigned ghost Mar 9, 2019
@markus2330
Copy link
Contributor

@Piankero what output do you suggest so that you would find the info about --?

@ghost
Copy link
Author

ghost commented Mar 9, 2019

I am not quite sure if this is possible but if the input was a number, then the text which tells you how to set negative numbers should be arranged at the top. The verbose output of a failed set could only be printed out for every other reason. As I can see there is no number option for kdb set so a solution like this should be fine.

Negative number:

kdb set '/my/number' '-3'                                          
#> kdb: invalid option -- '3'
#> Did you try to set a negative value? Try  'kdb set -- /tests/neg -3'
#> Sorry, I could not process the given options (see reason above)

Anything else:

kdb set '/invalid/option' '-a'                                                   
#> kdb: invalid option -- 'a'
#> Sorry, I could not process the given options (see errors above)
#> 
#> Usage: kdb set <name> [<value>]
#> 
#> Set the value of an individual key.
#> If no value is given, it will be set to a null-value
#> To get an empty value you need to quote like "" (depending on shell)
#> 
#> -H --help                Show the man page.
#> -p --profile <name>      Use a different profile for kdb configuration.
#> -v --verbose             Explain what is happening.
#> -q --quiet               Only print error messages.
#> -V --version             Print version info.
#> -N --namespace <ns>      Specify the namespace to use for cascading keys.
#> -C --color <when>       Print never/auto(default)/always colored output.

The reason for this is that settings will be very likely to have negative numbers. Most likely more often than people setting wrong options (which are more easy to debug than negative numbers)

@markus2330
Copy link
Contributor

Thank you for the proposal!

This would mean that we would need to check for this situation and yield a different error then.

@kodebach is this also easily possible with your command-line parser? I would like to avoid specific code that needs to be thrown after the switch.

@kodebach
Copy link
Member

Currently we treat everything starting with - as an option. It wouldn't be a huge problem to change this, but AFAIK this is standard POSIX operating procedure. If an option expects an argument and it is given as a separate element of argv (e.g. -o file or --output file) we don't treat leading dashes (-) as options. Also POSIX behaviour AFAIK.

While I don't mind changing behaviour to make user interactions more fluent, I don't like going against POSIX standards to do this.

One thing that would solve this problem is enabling posixly mode for KDB. By enabling this mode, we would stop option processing at the first non option argument (in this case the keyname). Personally I think we should use posixly anyways, for this and various other reasons (e.g. better sub commands).

@markus2330
Copy link
Contributor

Sorry, in my question it was not clear what "this" is. The proposal is to change the error message in the case somebody uses a -<number>, not to allow this.

@kodebach
Copy link
Member

Changing the error message should be possible as well. It would require modifications, but we could just set some metadata on the parent key, whenever an error is caused by an element of argv.

@markus2330
Copy link
Contributor

Then let us improve the error message for this case!

@ghost
Copy link
Author

ghost commented Nov 28, 2019

I am not sure what I should do here.

I can do the following points:

"see errors above" is misleading, "see lines above" might be better?
"If no value is given, it will be set to a null-value" and the following sentence does not have a dot at the end.

But I honestly do not want to do the complicated stuff that requires enabling posixly and incorporate special error message for different cases depending on the option provided. In retrospective I could have known that negative numbers in terminals always require a -- because there are some articles about it in general (at first I thought it is Elektra specific). So this was a mistake on my side.

Also I have seen that we actually support some sort of numeric parameters too:

if (acceptedOptions.find ('1') != string::npos)
{
option o = { "first", no_argument, nullptr, '1' };
long_options.push_back (o);
helpText += "-1 --first Suppress the first column.\n";
}
if (acceptedOptions.find ('2') != string::npos)
{
option o = { "second", no_argument, nullptr, '2' };
long_options.push_back (o);
helpText += "-2 --second Suppress the second column.\n";
}
if (acceptedOptions.find ('3') != string::npos)
{
option o = { "third", no_argument, nullptr, '3' };
long_options.push_back (o);
helpText += "-3 --third Suppress the third column.\n";
}

@markus2330 markus2330 unassigned ghost Nov 28, 2019
@markus2330
Copy link
Contributor

Let us close this issue. The usability is not so bad as the output even says what to do and even gives an example.

There is little we can do if people do not even read what is printed.

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

3 participants