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

Indicate type of value instead of <%s> in <name> values #492

Closed
eric-brechemier opened this issue Feb 14, 2017 · 6 comments
Closed

Indicate type of value instead of <%s> in <name> values #492

eric-brechemier opened this issue Feb 14, 2017 · 6 comments

Comments

@eric-brechemier
Copy link
Contributor

Currently, the same placeholder <%s> is given for all the command-line options, notwithstanding their type:

-output <%s>
-wrap <%s>
-language <%s>

It would be more useful to indicate the name of the type instead, for example:

-output file
-wrap number
-language lang
@geoffmcl
Copy link
Contributor

geoffmcl commented Mar 5, 2017

@eric-brechemier took me a bit of time to look into this... sorry...

Hmmm, these command-line options come from a specfically created table in tidy.c, and at present that table does not directly contain the type information, but that information does come as an internationalized string...

That same cmdopt_defs table is also used for the -h, -?, --help, ie the standard help option, where the %s is converted to a string, like you suggest... see the function print_help_option()...

So say -output <%s> is output as -output <file>, in English, and maybe -output <fichier>, in French, and -wrap <%s> becomes -wrap <column>, in English, and something else in french, german, etc, etc, etc...

It does this in localize_option_names(CmdOptDesc *pos), where, as the comment there states, "the option names aren't localized, but... <file> should be <archivo> in Spanish". It gets the local sub-key, tidyLocalizedString(pos->subKey);, and allocates memory and formats the complete string in tmbstr stringWithFormat( const ctmbstr fmt, ... )...

Now why is this not done when the output is in xml, from -xml-help? I guess just overlooked, or something...

Now this XML is done in the function xml_help(), print_xml_help_option(), print_xml_help_option_element("name", pos->name1)... Of course another wrinkle in XML output is that the < and > must also be escaped...

But there seems no reason why the same allocation with localized substitutions done could not also be done for XML... That tidy-help.xml is passed to xsltproc, to generate the man page... so this may need to also be considered...

I hope I have given enough clues as to where and how this is done in the normal help, so this could be applied to the XML output in the same, or similar way... if it presents no problem to xsltproc...

Look forward to comments, patches or even a PR to close this... thanks...

And it also provides some clues about #491... why in one case we have <name> with a dash, and not in the other - they are generated from completely different tables... but will now try to look deeper there, and see what can be done...

@balthisar
Copy link
Member

Hmmm.... I'll have to look into this, too. I've just realized that the man page, for example, indicates this:

       -output <%s>, -o <%s> (output-file: <%s>)
              write output to the specified <file>

That's pretty ugly, and I don't remember it always being that way... yet looking back at 5.2, yup, ugly as heck.

Appreciate a PR if you want to work on it, @eric-brechemier, otherwise I can take a look at it.

@balthisar
Copy link
Member

@eric-brechemier, take a look at the issue492 branch that's in the repo. It fixes this issue entirely. However, I'll leave this open because the XSLT still shows the ugly (output-file: <%s>) in the man page, and I don't have time to look at it right now. If you can improve the man, we can merge this in.

@geoffmcl
Copy link
Contributor

geoffmcl commented Mar 6, 2017

@balthisar well you changed the -output <%s> to -output <file>, but yes you did not apply the same changes, substitutions to the <eqconfig> entry, in tidy-help.xml...

Seems just a few more tiny steps ;=))

@balthisar
Copy link
Member

Yes, not quite ready for a PR yet, but that should be easy to fit in.

@balthisar
Copy link
Member

Actually, @geoffmcl's pointing out the eqconfig issue fixed the man issue, so there's nothing else to review. Merged, and closed.

balthisar added a commit that referenced this issue Mar 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants