[PATCH 1/2] fsutil: make usage and unsupported command messages localizable

[Vijay Kiran Kamuju]

split the original patch

Signed-off-by: Vijay Kiran Kamuju <infyquest(a)gmail.com>

[Zebediah Figura]

This patch subject is misleading, as it implies that these messages were 
already present. A better subject could be "fsutil: Print a usage 
message when called without arguments."

I also note that the usage message seems to be identical to Windows, 
minus unimplemented commands. In general we want to avoid copying 
Windows, only doing so when an application depends on it.

With respect to the implementation, I think some basic tests would not 
be a bad idea.

Some further comments:
> +    if (argc > 1)
> +    {
> +        FIXME("unsupported command %s\n", debugstr_w(argv[1]));
> +        output_string(STRING_UNSUPPORTED_CMD, argv[1]);
> +        ret = 1;
> +    }

I'm not sure it makes a lot of sense to output a localized message here; 
I think the FIXME is enough. Same for STRING_UNSUPPORTED_PARAM.
> +    LoadStringW(GetModuleHandleW(NULL), msg, fmt, sizeof(fmt)/sizeof(fmt[0]));

ARRAY_SIZE(fmt)

Also, I know that this is copied from elsewhere, but I imagine we could 
simplify this code by using FORMAT_MESSAGE_FROM_HMODULE.
> +        WINE_TRACE(" %s", wine_dbgstr_w(argv[i]));
> +    WINE_TRACE("\n");

I think, though I'm not sure, that TRACE and debugstr_w() are fine in 
programs nowadays.
> +static const WCHAR createW[]=L"create";
> +static const WCHAR hardlinkW[]=L"hardlink";

I don't think there's any reason not to just put these inline where 
they're used.
> +        if (!_wcsicmp(argv[2], createW))

This can be wciscmp, without the underscore.

[Arkadiusz Hiler]

On Thu, Apr 30, 2020 at 02:49:07PM -0500, Zebediah Figura wrote:
> > +    LoadStringW(GetModuleHandleW(NULL), msg, fmt, sizeof(fmt)/sizeof(fmt[0]));
>
> ARRAY_SIZE(fmt)
>
> Also, I know that this is copied from elsewhere, but I imagine we could 
> simplify this code by using FORMAT_MESSAGE_FROM_HMODULE.

This indeed looks line a neat improvement - we could get rid of
output_vsprintf() completetly with something like:

FormatMessageW(FORMAT_MESSAGE_FROM_HMODULE, NULL, msg, 0, out, ARRAY_SIZE(out), &arguments);


There is small difference in behavior though - LoadStringW() uses
STRINGTABLE from .rc whereas FormatMessageW(FORMAT_MESSAGE_FROM_HMODULE)
uses MESSAGETABLE from .mc file.

This would create a precedent among programs. Do you think it's okay to
go this way?
> > +        WINE_TRACE(" %s", wine_dbgstr_w(argv[i]));
> > +    WINE_TRACE("\n");
>
> I think, though I'm not sure, that TRACE and debugstr_w() are fine in 
> programs nowadays.

A lot of programs are using this, so I assume it's fine. At least
nothing blows up on my end.

-- 
Cheers,
Arek