davetron5000 / gli

Make awesome command-line applications the easy way
http://davetron5000.github.io/gli
Apache License 2.0
1.26k stars 102 forks source link

Long form switch/flag names don't work in config files #242

Closed peter-ellis closed 3 years ago

peter-ellis commented 8 years ago

When a switch or flag has both a short form (e.g, -a) and a long form (e.g., --annotate) option name, only the short form appears to work when used in the config file; the long form is ignored. When there is only a long form, it works okay in the config file.

It seems like either short or long form names should work in the config file. If anything, maybe there should be a preference for long form names for the sake of readability.

I think I tracked the problem down to the override_default method in AppSupport. The issue is that the token 'name' is just the short form name when both short and long exist.

I have a fix that seems to work for me, but is not maybe pull request worthy. So here it is for your consideration:

def override_default(tokens, config)
  tokens.each do |name, token|
    all_aliases = [token.name]
    all_aliases += token.aliases if (token.aliases)
    matches = config.keys & all_aliases  # Could have multiple aliases in config file, i.e., over specified
    if not matches.empty?
      token.default_value = config[matches.last]  # Use last match; should be last occurrence in config file, but not necessarily
    end
  end
end
pboling commented 8 years ago

This would have been very confusing for me, and I was about to run headlong into it. Thanks for the report! 👍

davetron5000 commented 8 years ago

Yeah, the intention is for either to work. At one point there was a bug where initconfig would write both (or all) versions of a flag into the config file, thus making it appear to work with tests.

There are a few tests for this:

They use this test config: https://github.com/davetron5000/gli/blob/gli-2/test/config.yaml

Is that enough for you to write a failing test? That would be super helpful is sorting this out.

davetron5000 commented 3 years ago

Consolidated into #305