findutils icon indicating copy to clipboard operation
findutils copied to clipboard

use uutils-args or clap for argument management

Open sylvestre opened this issue 4 years ago • 6 comments

findutils is doing way too many argument management by hand. It should be delegate to clap for example https://github.com/uutils/findutils/blob/ac576f50b0acedf59a7d64144787fa3129c635bd/src/find/mod.rs#L89-L92 https://github.com/uutils/findutils/blob/ac576f50b0acedf59a7d64144787fa3129c635bd/src/find/mod.rs#L167-L175 https://github.com/uutils/findutils/blob/ac576f50b0acedf59a7d64144787fa3129c635bd/src/find/mod.rs#L205-L207 https://github.com/uutils/findutils/blob/ac576f50b0acedf59a7d64144787fa3129c635bd/src/find/matchers/mod.rs#L205-L212

sylvestre avatar Sep 07 '21 20:09 sylvestre

I've got no problems with that (I don't think clap was available when I wrote the initial version). But are you sure clap is up to the task? Most command-line parsers I've seen say "here are the validated options, with their associated values if any, and here are the remaining arguments. They don't care which order the options came in, or how they were interleaved with the arguments.

That's not the case with find:

find -false ( -not -false -o -true ) find -false ( -false -o -not -true)

have the same "options" but their order is different (and the result of the command should be too)

similarly with

find -true ( -not -false -o -true ) find -true -not ( -false -o -true )

the relative positioning of the options and the brackets (which I'd assume would count as "arguments", rather than "options" is important.

If clap can handle that, then sure go ahead and simplify the code. But before you start make sure that clap can correctly handle, e.g.

  • all the nested brackets, (implied and explicit) ands, ors and commas as discussed above and
  • all the options passed to sub-commands between "-exec" and ";" or "+"

Hope that makes sense

Mark

On Tue, Sep 7, 2021 at 9:56 PM Sylvestre Ledru @.***> wrote:

findutils is doing way too many argument management by hand. It should be delegate to clap for example

https://github.com/uutils/findutils/blob/ac576f50b0acedf59a7d64144787fa3129c635bd/src/find/mod.rs#L89-L92

https://github.com/uutils/findutils/blob/ac576f50b0acedf59a7d64144787fa3129c635bd/src/find/mod.rs#L167-L175

https://github.com/uutils/findutils/blob/ac576f50b0acedf59a7d64144787fa3129c635bd/src/find/mod.rs#L205-L207

https://github.com/uutils/findutils/blob/ac576f50b0acedf59a7d64144787fa3129c635bd/src/find/matchers/mod.rs#L205-L212

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/uutils/findutils/issues/113, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC3MQF6OLF5L73BBQQON7TDUAZ36TANCNFSM5DTFRHOA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

mcharsley avatar Sep 08 '21 08:09 mcharsley

Yeah, indeed, not a trivial task for sure! I think we can delegate the easy stuff to clap and parse the complex stuff by hand

sylvestre avatar Sep 08 '21 17:09 sylvestre

or use https://github.com/uutils/uutils-args/ cc @tertsdiepraam

sylvestre avatar Feb 11 '24 14:02 sylvestre

Uutils-args can't do this at the moment, I think, but we can probably extend it somehow to do it. Actually, the whole syntax tree might be difficult to implement. I'll have to look into it. It reminds me of the parsing I implemented for env

tertsdiepraam avatar Feb 11 '24 14:02 tertsdiepraam