Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parameter values are converted to lowercase when passed on the command line #86

Open
AndyGee opened this issue Feb 3, 2020 · 6 comments

Comments

@AndyGee
Copy link
Contributor

AndyGee commented Feb 3, 2020

Command line prameter is "--targetLocale=zh_TW" (Mixed case value)

@option("targetLocale") final Locale targetLocale //Preferred method
@option("targetLocale") final String targetLocale //Tested, but same result

Either option produces a lowercase value "zh_tw" - So the command line parameter case is NOT honoured.

Locale z = new Locale("zh_zw"); is not validated, but is also NOT a valid locale.
org.apache.commons.lang3.LocaleUtils.toLocale(z.toString()); is validated and throws an error.

The command and value case must not be converted to lowercase.

@rmannibucau
Copy link
Contributor

Hi @AndyGee ,

I don't think it is crest by itself, typically the second flavor (using a String) passthrough the case.
The issue with the Locale is the usage of Locale(String) implicitly because there is no property editor of String->Locale and this constructors does not parse the locale properly (this is the one doing the tolowercase).

Typically this test passes:

public class SampleTest {
    @Test
    public void run() throws Exception {
        PropertyEditorManager.registerEditor(Locale.class, LocaleEditor.class);
        assertEquals(
                "zh_TW/zh_TW",
                new Main(Commands.class).exec("locales", "--locale=zh_TW", "--locale-string=zh_TW"));
    }

    public static class Commands {
        @Command
        public String locales(@Option("locale") final Locale locale,
                              @Option("locale-string") final String localeString) {
            return locale.toString() + "/" + localeString;
        }
    }

    public static class LocaleEditor extends PropertyEditorSupport {
        @Override
        public void setAsText(final String s) {
            final String[] segments = s.split("_");
            switch (segments.length) {
                case 1:
                    setValue(new Locale(s));
                    break;
                case 2:
                    setValue(new Locale(segments[0], segments[1]));
                    break;
                case 3:
                    setValue(new Locale(segments[0], segments[1], segments[2]));
                    break;
                default:
                    throw new IllegalArgumentException(s);
            }
        }
    }
}

@AndyGee
Copy link
Contributor Author

AndyGee commented Feb 3, 2020 via email

@AndyGee
Copy link
Contributor Author

AndyGee commented Feb 3, 2020 via email

@rmannibucau
Copy link
Contributor

Hmm, it behaves exactly the same for me. The only difference is you can't register the editor as in the test and must register it before the command parameters are instantiated.
You can either wrap the crest main to do that or use its Loader API (with its META-INF/services/org.tomitribe.crest.cmds.processors.Commands$Loader) if it is how you register commands:

public class Registrar implements Commands.Loader {
    @Override
    public Iterator<Class<?>> iterator() {
        PropertyEditorManager.registerEditor(Locale.class, LocaleEditor.class);
        return Collections.<Class<?>>singleton(LocaleCmd.class).iterator();
    }
}

@AndyGee
Copy link
Contributor Author

AndyGee commented Feb 4, 2020 via email

@AndyGee
Copy link
Contributor Author

AndyGee commented Feb 4, 2020 via email

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

No branches or pull requests

2 participants