Use OptionParser.parse #193
Conversation
we have to specify option strict: with empty list,
otherwise we get an error from elixir:
"warning: not passing the :switches or :strict option to OptionParser is deprecate"
parse component as first position argument from CLI. Rest of positional arguments are unused. This goal is achieved by pattern matching of private func get_component. If no positional argument is specified then "Downloader" must be used.
lib/cli.ex
Outdated
| def main([component]) do | ||
| module = Module.safe_concat("Onigumo", component) | ||
| def main(argv) do | ||
| {_parsed, args, _invalid} = OptionParser.parse(argv, strict: []) |
There was a problem hiding this comment.
If we match the invalid arguments with [], we‘ll get a MatchError. Like that, we can leverage the current rescue clause catching for OptionParser.ParseError and MatchError. It’s not something we want to keep as we want to abandon raising exceptions (see #204), but it can be a nice step that keeps a minimal amount of changes.
The same applies for the parsed argument, which we already do in the codebase. They both would raise a MatchError and we currently don’t need to differentiate between them.
| {_parsed, args, _invalid} = OptionParser.parse(argv, strict: []) | |
| {[], args, []} = OptionParser.parse(argv, strict: []) |
lib/cli.ex
Outdated
| root_path = File.cwd!() | ||
| module.main(root_path) | ||
| end | ||
|
|
||
| defp get_component([]), do: "Downloader" |
There was a problem hiding this comment.
This adds additional logic of setting a default component. That’s not necessarily wrong, at least now when there is only a single component, but it’s over the scope of introducing OptionParser.
lib/cli.ex
Outdated
| module = | ||
| args | ||
| |> get_component | ||
| |> safe_concat |
There was a problem hiding this comment.
After a productive discussion, we abandoned safe_concat in favor of a module literal.
lib/cli.ex
Outdated
| defp safe_concat(component) do | ||
| Module.safe_concat("Onigumo", component) | ||
| end |
There was a problem hiding this comment.
As mentioned above. Thanks to abandoning safe_concat as unsafe 😃, exposing new errors and abusing reflection, we won’t need this function altogether.
| defp safe_concat(component) do | |
| Module.safe_concat("Onigumo", component) | |
| end |
nappex
force-pushed
the
use-optionParser.parse
branch
from
6dbb778 to
f14d087
Compare
| @@ -5,10 +5,10 @@ defmodule Onigumo.CLI do | |||
|
|
|||
| def main(argv) do | |||
| try do | |||
| {[], [component]} = OptionParser.parse!(argv, strict: []) | |||
| {[], [component], []} = OptionParser.parse(argv, strict: []) | |||
There was a problem hiding this comment.
Just mentioning that in this case, there are now two different MatchErrors that can occur:
- An invalid switch is provided, causing the third assignment
[]to not match. - More than one positional argument causing the secund assignment
[component]to fail.
We don’t, however, need to differentiate these two, as both mean invalid CLI usage.
fix #204