Skip to content

started moving to optparse-applicative#400

Closed
theotherjimmy wants to merge 4 commits intofosskers:aura-1.4from
theotherjimmy:optparse-applicative
Closed

started moving to optparse-applicative#400
theotherjimmy wants to merge 4 commits intofosskers:aura-1.4from
theotherjimmy:optparse-applicative

Conversation

@theotherjimmy
Copy link
Contributor

WIP.
Questions:

  • At the moment, every command line possible parses successfully, leaving aura.hs to interperate the 'pacOpts' flags. Should we instead parse all of pacman's options and allow the parser to fail when some flag matches neither pacman options nor aura options?

More to come as I think of them.
@fosskers, could you comment on the question above?

@theotherjimmy
Copy link
Contributor Author

Oh, I just remembered, I don't think this is supposed to work this way:

haskell/aura [ ./dist/build/aura/aura -Aabcdefghijklmnopqrstuvwxyz                                          ]  1 (8:03 PM)
Success (([AURInstall,DelMDeps,PacmanArg "dbpath" "cdefghijklmnopqrstuvwxyz"],[]),[])
aura >>= You have to use `sudo` for that.

@fosskers
Copy link
Owner

No, that's evil. I wonder if that's a bug in the library?

@theotherjimmy
Copy link
Contributor Author

I'll take a look and submit an issue if I need to.
On Mar 14, 2016 7:43 PM, "Colin Woodbury" notifications@github.com wrote:

No, that's evil. I wonder that's a bug in the library?


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#400 (comment)

@theotherjimmy
Copy link
Contributor Author

I just checked, and that is the same behavoir as pacman. eg.

haskell/aura [ pacman -bcdef                                          ]  255 (8:14 PM)
error: failed to initialize alpm library
(could not find or read directory: cdef)

@fosskers
Copy link
Owner

I think I see what's happening here. If a short-op takes an arg, it tries to parse as much as possible out of that. Notice -y is a legal suboption for -A, but we see it parsed an argument to -b.

@theotherjimmy
Copy link
Contributor Author

I think it's done, but I would need to test all options. That would suck.

@fosskers
Copy link
Owner

Haha very much so. I'm going to heavily review this as well.

@theotherjimmy
Copy link
Contributor Author

Just noticed, things like -Q don't work.

@theotherjimmy
Copy link
Contributor Author

This just might work.

@theotherjimmy
Copy link
Contributor Author

Looks good to me. It seems to do what you would expect: parse it's own commands and, if that fails, hand it off to pacman. Lets get this merged. @fosskers ?

@fosskers fosskers mentioned this pull request May 30, 2018
12 tasks
@fosskers fosskers closed this Jun 26, 2018
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

Successfully merging this pull request may close these issues.

2 participants