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

Mention the need for a : before options' value in the configuration file #698

Closed
doronbehar opened this issue Mar 18, 2018 · 9 comments
Closed
Labels
Milestone

Comments

@doronbehar
Copy link
Contributor

As the title suggests, I would like to offer a little improvement to the man page. On lines 75 - 78, it isn't mentioned that one has to use : in order to assign value to an option on the configuration file.

I was trying to understand why my configuration file is not read with $HTML_TIDY and after looking at some closed issues here on GitHub I realised I'm missing a : before a configuration value. I think it should be noted on the man page.

I didn't make a PR because I'm not sure I know good enough how to write with a man page format.

@geoffmcl
Copy link
Contributor

@doronbehar thank you for the issue...

While I agree it is not specifically mentioned in those lines 75-78 that one has to use a : in order to assign a value, there are many, MANY samples given, a little further down, even a sample config file...

Down a few lines at 91 is says "as they would appear in a configuration file" and the example (\fIquiet: yes\fR)... with a colon...

And each option given in the man tidy pages that has a configuration file equivalent it is list with the colon, like the first -output <file>, -o <file> (output-file: <file>), and so on for each option, with an equivalent...

Then down at line 114 it say "each option is listed on a separate line in the form" and two examples are given showing option: value... with a colon...

And down at line 141 a sample configuration file is given with each of the five types, each option followed by a colon...

And then there is the config.c source in function ParseConfigFileEnc...

However if you have a suggested addition, improvement, to the wording of that template file, give it and it will certainly be considered... as a comment, a patch, here, or a PR...

We certainly agree the documentation can always be enhanced... thanks...

@geoffmcl geoffmcl added the Docs label Mar 18, 2018
@geoffmcl geoffmcl added this to the 5.7 milestone Mar 18, 2018
@doronbehar
Copy link
Contributor Author

Hmm, I wonder how I missed that.. Although the examples are there I still think it should be stated explicitly.

@geoffmcl
Copy link
Contributor

@doronbehar well I asked for a suggestion... oh, well...

How about?

diff --git a/man/tidy1.xsl.in b/man/tidy1.xsl.in
index 0fbae79..3efc31d 100644
--- a/man/tidy1.xsl.in
+++ b/man/tidy1.xsl.in
@@ -75,8 +75,8 @@ They are listed in the first part of this section.
 \fIConfiguration\fR options, on the other hand, can either be passed
 on the command line, starting with two dashes \fB--\fR,
 or specified in a configuration file,
-using the option name without the starting dashes.  
-They are listed in the second part of this section.
+using the option name, followed by a colon \fB:\fR, plus the value, without 
+the starting dashes. They are listed in the second part of this section.
 .LP
 For \fIcommand-line\fR options that expect a numerical argument,
 a default is assumed if no meaningful value can be found.  

Or do you have an alternative, perhaps better suggestion... thanks...

@doronbehar
Copy link
Contributor Author

doronbehar commented Mar 19, 2018

Hmm sorry for not adding a PR or patch my self, I wasn't around a computer. Your diff looks great but allow me to suggest something similar but with an example as well (I'm not sure about the formatting), here it is:

diff --git a/man/tidy1.xsl.in b/man/tidy1.xsl.in
index 0fbae79..3efc31d 100644
--- a/man/tidy1.xsl.in
+++ b/man/tidy1.xsl.in
@@ -75,8 +75,8 @@ They are listed in the first part of this section.
 \fIConfiguration\fR options, on the other hand, can either be passed
 on the command line, starting with two dashes \fB--\fR,
 or specified in a configuration file,
-using the option name without the starting dashes.  
-They are listed in the second part of this section.
+using the option name, followed by a colon \fB:\fR, plus the value, without 
+the starting dashes. They are listed in the second part of this section.
+Here's an example:
+    quite: yes
+    indent: yes
+    output-xhtml: yes
 .LP
 For \fIcommand-line\fR options that expect a numerical argument,
 a default is assumed if no meaningful value can be found.  

@geoffmcl
Copy link
Contributor

@doronbehar thank for the feedback, but now you have introduced some redundancy, and a small spelling mistake quite vs. quiet.

The file already contains a full sample, with a comment example, and each of the five types - see L141-L153.

So maybe we mention that, with -

diff --git a/man/tidy1.xsl.in b/man/tidy1.xsl.in
index 0fbae79..a1d9cfd 100644
--- a/man/tidy1.xsl.in
+++ b/man/tidy1.xsl.in
@@ -75,8 +75,9 @@ They are listed in the first part of this section.
 \fIConfiguration\fR options, on the other hand, can either be passed
 on the command line, starting with two dashes \fB--\fR,
 or specified in a configuration file,
-using the option name without the starting dashes.  
-They are listed in the second part of this section.
+using the option name, followed by a colon \fB:\fR, plus the value, without 
+the starting dashes. They are listed in the second part of this section,
+with a sample config file.
 .LP
 For \fIcommand-line\fR options that expect a numerical argument,
 a default is assumed if no meaningful value can be found.  

What do you think? Are we good to go? Thanks...

@doronbehar
Copy link
Contributor Author

The sample is not present in the stable version, I missed that when I added an example of my own.

So I guess yes, we are good to go, thank you for your collaboration!

@geoffmcl
Copy link
Contributor

@doronbehar not sure what you mean by "not present in the stable version"? I checked and the sample config file is present in the master ie. release/5.6, and in release/5.4, release/5.2, and maybe earlier than that...

But no matter, we agree, and have created an issue-698 branch with this doc fix...

Will get around to creating a PR to pull this into next...

Yes, thanks for your collaboration...

Hope you find more places where the docs can be improved, as they always can be... thanks...

@doronbehar
Copy link
Contributor Author

I guess I missed that as well :/.. Thanks a lot!

geoffmcl added a commit that referenced this issue Mar 27, 2018
geoffmcl added a commit that referenced this issue Apr 19, 2018
geoffmcl added a commit that referenced this issue Apr 19, 2018
@geoffmcl
Copy link
Contributor

Now PR #702 now merged, so closing this... thanks @doronbehar ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants