Refactor CLI Enums to use clap::ValueEnum

[tareknaser]

This change automatically prints the possible values to the user in the help message

[kirillt]

@tareknaser thanks, it's better. But actually would be great to implement slightly different logic for --id and --path options. I think we could remove --both and just allow typing -ip instead of -b, it seems to be more intuitive this way.

[tareknaser]

I think we could remove --both and just allow typing -ip instead of -b, it seems to be more intuitive this way.

Updated
Also, made this change for other CLI enums
Let me know if you want any change to the naming of parameters

I was not clear enough about -ip

[kirillt]

Sorry, I meant a bit different handling of --entry option.

The user can write --entry=id or --entry=path. I suggest refactor these long variants of the option into something simple but explicit like --id and --path. User may pass --id --path to print both fields.

And we should support short option -ip. So, the user could type -i or --id instead of --entry=id. Also, -p or --path instead of --entry=path.

Finally, -ip should be shorthand for -i -p or --id --path. This is inspired by UNIX shell commands, e.g. ls -lah is same as ls -l -a -h.

Is this straightforward to implement using clappy?

[tareknaser]

Alright, I just noticed that ark-lib list has different options to set view mode:

• --entry <ENTRY>      [possible values: id, path, ip]
• -i, --entry-id
• -p, --entry-path
  We manually check if the user provided more than one of them and return an error in main.rs

Here are the changes I made to implement the structure you mentioned:

• Completely remove --entry <ENTRY> option
• -i or --id to only show ids
• -p or --path to only show paths
• -i -p or --id --path or -ip to show both

We will also refactor the CLI parameter processing to be handled within a method of the Command enum. This way, the code in main.rs can directly access the processed values.

Let me know what you think

[kirillt]

@tareknaser

Here are the changes I made to implement the structure you mentioned:

• Completely remove --entry <ENTRY> option

That's ok. I think --entry-id and --entry-path provide same convenience.

• -i or --id to only show ids
• -p or --path to only show paths
• -i -p or --id --path or -ip to show both

Exactly what is needed, thanks.

[kirillt]

We will also refactor the CLI parameter processing to be handled within a method of the Command enum. This way, the code in main.rs can directly access the processed values.

Do you want to do it in this PR or just create an issue for this moment?

[kirillt]

I've discovered this minor issue:

$ ark-cli list -ip

57-2877975594@/tmp/test/57-2877975594
180179-1694990742@/tmp/test/truth.jpg
106-699027850@/tmp/test/github.com-106-699027850.link
57-2158444521@/tmp/test/github.com-57-2158444521.link

$ ark-cli list -ip

180179-1694990742@/tmp/test/truth.jpg
57-2877975594@/tmp/test/57-2877975594
57-2158444521@/tmp/test/github.com-57-2158444521.link
106-699027850@/tmp/test/github.com-106-699027850.link

The output is not deterministic... It can be important when the output is used in pipe, the user could run one command, then craft very specific commands which should work with the particular output. If it changes, then the user's commands would not function correctly since they rely on the order.

We have deterministic output when using list command without parameters:

$ ark-cli list

180179-1694990742
57-2877975594
106-699027850
57-2158444521

$ ark-cli list

180179-1694990742
57-2158444521
106-699027850
57-2877975594

[tareknaser]

Do you want to do it in this PR or just create an issue for this moment?

I have it implemented for this particular parameter (Command.entry()). We can implement it for other parameters if needed as well.
For example, I think we can implement it for Command::Render quality

[tareknaser]

The output is not deterministic

It appears to be not deterministic regardless of the flags provided. ark-cli list gave 2 different outputs in the example you mentioned.

Tracing this back, I don't think this is related to ark-cli. seems to have originated from ark-lib.
I suspect the reason for this is coming from ResourceIndex::provide() but I would have to spend some more time digging into it.

[kirillt]

@tareknaser indeed, I didn't notice the ids starting with 57 swap places in the ark-cli list output.

[kirillt]

@tareknaser Could you create the follow-up issues? Feel free to merge after conflicts resolution.

[tareknaser]

I rebased this PR on top of main but I think it needs another review because some of the changes here conflicted with changes from #27

In #27, a new formatting option ("link") was added for the output of ark-cli list. I kept the same logic here in this PR but moved command line parameters handling to cli.rs instead of main.rs

[tareknaser]

rebased on top of main but let's wait for ARK-Builders/ark-core#13 first to be merged and move this PR to ark-rust

[tareknaser]

Done in ARK-Builders/ark-core#25