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

Reject nbgv get-version "" #969

Open
KalleOlaviNiemitalo opened this issue Aug 11, 2023 · 1 comment
Open

Reject nbgv get-version "" #969

KalleOlaviNiemitalo opened this issue Aug 11, 2023 · 1 comment

Comments

@KalleOlaviNiemitalo
Copy link

KalleOlaviNiemitalo commented Aug 11, 2023

I'd like nbgv get-version "" to return an error, like git tag does:

$ git tag hah ""
fatal: Failed to resolve '' as a valid ref.

In nbgv 3.5.108+6e793d63d3, both nbgv get-version and nbgv get-version "" mean the same as nbgv get-version HEAD.

Rationale

To work around nbgv not supporting all the same <commit-ish> syntax as Git, my script preprocessed the argument like nbgv get-version "$(git rev-parse "${rev}")". But when ${rev} was accidentally empty, the script executed nbgv get-version "", which displayed the version information of HEAD instead. If that had returned an error right away, it would have finished faster (nbgv get-version HEAD takes 1500 ms in my repo) and not output misleading information.

Implementation ideas

nbgv treats an empty string as equivalent to null here:

if (string.IsNullOrEmpty(commitish))
{
commitish = DefaultRef;
}

Could either tighten that check, or add a validator for the argument here:

new Argument<string>("commit-ish", () => DefaultRef, $"The commit/ref to get the version information for.")
{
Arity = ArgumentArity.ZeroOrOne,
},

@AArnott
Copy link
Collaborator

AArnott commented Aug 11, 2023

I suppose nbgv get-version "" can be made to error out, provided nbgv get-version still works.
I'd favor rejecting "" at the command parsing level rather than in the command handler.

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

No branches or pull requests

2 participants