Markbind help: update output to match userguide #550

JYL123 · 2019-01-05T07:40:27Z

What is the purpose of this pull request? (put "X" next to an item, remove the rest)

• [X ] Enhancement to an existing feature

What is the rationale for this request?
Fixes #535

What changes did you make? (Give an overview)
Update index.js command descriptions to match the updates in user guide. The modified command overview is as follows from terminal:

Is there anything you'd like reviewers to focus on?

Testing instructions:
Run markbind -h from terminal

Xenonym · 2019-01-05T10:57:34Z

index.js

-  .option('-p, --port <port>', 'port for server to listen on (Default is 8080)')
-  .option('-s, --site-config <file>', 'specify the site config file (default: site.json)')
+  .option('-p <port>, --port <port>', 'port for server to listen on (Default is 8080)')
+  .option('-s <file>, --site-config <file>', 'specify the site config file (default: site.json)')


This change breaks the options: -p and -s do not have an effect, and the long options --port and --site-config are not recognised.

The reason is because the library we are using for the CLI, commander.js, parses the first argument to .option as a list of flags:

The flags string should contain both the short and long flags, separated by comma, a pipe or space.

I don't think we can make this specific change without breaking the CLI.

Hi @Xenonym, thanks for the explanation. I should have tested it myself. I got it now. Then I will remove my modification.

Xenonym · 2019-01-05T10:58:35Z

index.js

@@ -172,6 +172,7 @@ program

 program
  .command('build [root] [output]')
+  .option('<output>', 'put the generated files in the specified directory')


As with earlier, commander.js expects a list of flags as the first argument to .option(). Passing in anything else doesn't seem to have meaningful results.

Thanks for pointing out again. I will remove this line as well. For this issue, is there any other parts that i have missed which is supposed to be done?

Xenonym · 2019-01-05T14:44:13Z

@JYL123 comparing between the user guide and the output of markbind <command> -h, these are the differences I notice:

The commands are listed in the user guide in alphabetical order, but not so in markbind. We could alphabetise the commands in index.js to match the user guide.
Optional arguments are denoted with [] in markbind eg. --baseUrl [baseUrl] but marked with <> in the user guide. Since we cannot change the flag description without changing the behaviour of the CLI, we should probably change the user guide to match instead.

yamgent · 2019-01-06T02:55:56Z

@Xenonym excellent review, good work. 👍

@damithc just checking, how much of the matching were you looking for when you created #535? Were you expecting a close match like this:

$ markbind --help

  ...

  Commands:

  build [options] [root] [output]  Generates the site to the directory named _site
                                   in the current directory.
  deploy                           Deploys the site to the repo's Github pages by 
                                   pushing everything in the generated site (default 
                                   dir: _site) to the gh-pages branch of the current 
                                   git working directory's remote repo. 

  ...

damithc · 2019-01-06T03:28:25Z

@damithc just checking, how much of the matching were you looking for when you created #535?

It's better to be as close as possible, right? For example, use the same notation for options markbind build [<OPTIONS>].
Perhaps we can standardize similar to javadocs. Example: In the user guide, the first paragraph of the description is 1-2 sentence overview of the command/parameter that will be reused in the CLI help. Further details can be in subsequent paragraphs.

Xenonym · 2019-01-06T03:45:20Z

It's better to be as close as possible, right? For example, use the same notation for options markbind build [<OPTIONS>].

The description in the CLI is actually parsed by commander.js to determine what arguments the command takes. I don't think we can change it without breaking the commands.

damithc · 2019-01-06T04:07:45Z

The description in the CLI is actually parsed by commander.js to determine what arguments the command takes. I don't think we can change it without breaking the commands.

I see. I don't mind if the user guide is changed to match the format used by CLI, if that format is good enough. In the user guide, I was trying to follow the format used by Git documentation.

JYL123 · 2019-01-06T05:18:18Z

Hi, thanks for clarifying this issue. is it okay that I make the following changes:

for user guide, refer to this Git documentation, can I remove <> for OPTIONS, root, output and leave the rest as it is? because from the documentation, I see <> is used for extra values added within []. So for example, my imagination for build is :

build Command
Format: markbind build [OPTIONS]

OPTIONS:
* output
...
* root output
...
* --baseUrl <base>
...

for index.js, I will add Description from user guide for each command description, and add option descriptions from user guide, such as <output> Put the generated files in the specified directory, in flag descriptions.

Please correct me if anything is wrong or not appropriate. Thanks in advance.

damithc · 2019-01-06T06:20:58Z

because from the documentation, I see <> is used for extra values added within [].

It looks like <> is used to indicate a placeholder where a value is expected. e.g., <root> means it it expects a value such as c:\ whereas options without <> e.g., --base-url are keywords that should be repeated exactly as they are. What do you think?

JYL123 · 2019-01-06T09:27:13Z

Thanks prof. I see. It makes sense. Then I think the user guide is good. I will undo my change to the user guide, namely keep the user guide as follows:

Format:  markbind build [<OPTIONS>]
...
OPTIONS:
* <output>
...
* <root> <output>
...

And change the descriptions in index.js as the comment above.

damithc · 2019-01-06T10:12:38Z

Thanks prof. I see. It makes sense. Then I think the user guide is good. I will undo my change to the user guide, namely keep the user guide as follows:
Format:  markbind build [<OPTIONS>]
...
OPTIONS:
* <output>
...
* <root> <output>
...
And change the descriptions in index.js as the comment above.

Got it. I'll leave the senior devs to make the final decision w.r.t. the exact format to follow.

yamgent · 2019-01-07T03:48:23Z

for index.js, I will add Description from user guide for each command description, and add option descriptions from user guide, such as <output> Put the generated files in the specified directory, in flag descriptions.

Is that actually possible with commander.js? I don't see a way of specifying the description for the compulsory arguments...

Also, I am in favour of such a format:

Example: In the user guide, the first paragraph of the description is 1-2 sentence overview of the command/parameter that will be reused in the CLI help. Further details can be in subsequent paragraphs.

Basically, the help description in the CLI should be as short as possible (which is what we have currently). The user guide should also have the short description, but then it will be followed by the details for the command.

For example, the help description for markbind build should be a single sentence summary:

$ markbind build --help

  Usage: build [options] [root] [output]

  Builds the website


  Options:

But the user guide should contain the single sentence summary + additional description of what it does:

`build` Command

Format: markbind build [<OPTIONS>]

Description: Builds the website.

Generates the site to the directory named _site in the current directory.

...

EDIT: Edited a bit of the format.

yamgent

For markbind build, update the user guide's description of --baseUrl to match what you have here (e.g. in the user guide, it mentions "with the give" rather than "with the given").
For markbind serve, update the description of the options in index.js to the first statement in the user guide.
Please add the following code, such that when markbind --help is invoked, it says "Usage: markbind " instead of "Usage: index ":

diff --git a/index.js b/index.js
index 33ccb4f..8bd0007 100755
--- a/index.js
+++ b/index.js
@@ -29,6 +29,7 @@ function printHeader() {
 }

 program
+  .name('markbind')
   .allowUnknownOption()
   .usage(' <command>');

yamgent · 2019-01-08T05:33:02Z

docs/userGuide/cliCommands.md

@@ -65,7 +69,9 @@ MarkBind Command Line Interface (CLI) can be run in the following ways:

 **Format:** `markbind init [<OPTIONS>]`

-**Description:** Initializes a directory into a MarkBind site by creating a skeleton structure for the website which includes a `index.md` and a `site.json`.
+**Description:** Init a markbind website project.


markbind -> MarkBind. (this should also be modified in index.js)

yamgent · 2019-01-08T05:38:26Z

docs/userGuide/cliCommands.md

@@ -82,7 +88,9 @@ MarkBind Command Line Interface (CLI) can be run in the following ways:

 **Format:** `markbind serve [<OPTIONS>]`

-**Description:** Does the following steps:
+**Description:** Build then serve a website from a directory.


~~Build then~~ serve a website from a directory. (this should also be modified in index.js)

While it is true that MarkBind does build the site into _site/, techincally, they don't really have to do that (i.e. we could have built the files in a temporary hidden directory rather than modifying _site/, because markbind serve is not really supposed to do building anyway). Therefore, I stroke off the "Build then" portion because that is too much unnecessary details.

The steps that follow (i.e. the "Does the following steps:" part) can stay however.

yamgent · 2019-01-08T05:41:52Z

index.js

+
+program
+  .command('deploy')
+  .description('deploy the site to the repo\'s Github pages')


Github -> GitHub. site -> website. (this should also be modified in cliCommands.md)

yamgent · 2019-01-08T06:14:28Z

There are two other issues that I have spotted.

<arg>: compulsory argument. [arg] = optional argument. The user guide is not consistent on this front.
Options are different from arguments. Options have a flag (e.g. -n or --no-open). Arguments do not have a flag. The user guide is not making the distinction clear.

JYL123 · 2019-01-08T10:31:14Z

@yamgent Thanks for the review!

yamgent

We don't use merge commits inside PR (we want to keep the repo history as clean as possible). So the merge commit Merge branch 'master' into update-help-to-match-userguide shouldn't appear in this PR.

A rebase should be done to resolve conflicts instead. See this guide for details.

yamgent · 2019-01-09T05:38:59Z

docs/userGuide/cliCommands.md

@@ -69,7 +69,7 @@ Deploys the site to the repo's Github pages by pushing everything in the generat

 **Format:** `markbind init [<OPTIONS>]`

-**Description:** Init a markbind website project.
+**Description:** Init a Markbind website project.


The B needs to be capitalized as well, that is how we brand our project. (i.e. MarkBind, not Markbind)

yamgent · 2019-01-09T05:49:54Z

docs/userGuide/cliCommands.md

+* `-o, --one-page [file]`<br>
+   Render and serve only a single page from your website.<br>
+  {{ icon_example }} `--one-page guide/index.md`
+* `-p [port]`, `--port [port]`<br>


You got it the wrong way round. :P

Things like [root] are the optional arguments (users can omit root when using markbind serve). They should be wrapped with [].

Things like <port> are the compulsory arguments (users must specify a port number when using -p). They should be wrapped with <>.

Most of the commands and options have this issue as well, please do rectify them, thanks!

update markbind help to match userguide

0939908

Xenonym reviewed Jan 5, 2019

View reviewed changes

update user guide and alphabetise commands

9b79ce1

JYL123 added 3 commits January 7, 2019 15:06

use short descriptions

6921f61

update user guide

7a68ff8

remove full stop

9a054fd

yamgent requested changes Jan 8, 2019

View reviewed changes

JYL123 closed this Jan 8, 2019

JYL123 reopened this Jan 8, 2019

yamgent requested changes Jan 9, 2019

View reviewed changes

JYL123 added 2 commits January 10, 2019 12:05

update command descriptions

335939c

update user guide

d7bd50b

JYL123 closed this Apr 22, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Markbind help: update output to match userguide #550

Markbind help: update output to match userguide #550

JYL123 commented Jan 5, 2019 •

edited

Xenonym Jan 5, 2019

JYL123 Jan 5, 2019

Xenonym Jan 5, 2019

JYL123 Jan 5, 2019

Xenonym commented Jan 5, 2019

yamgent commented Jan 6, 2019

damithc commented Jan 6, 2019

Xenonym commented Jan 6, 2019

damithc commented Jan 6, 2019

JYL123 commented Jan 6, 2019

damithc commented Jan 6, 2019

JYL123 commented Jan 6, 2019

damithc commented Jan 6, 2019

yamgent commented Jan 7, 2019 •

edited

yamgent left a comment

yamgent Jan 8, 2019

yamgent Jan 8, 2019

yamgent Jan 8, 2019

yamgent commented Jan 8, 2019

JYL123 commented Jan 8, 2019

yamgent left a comment

yamgent Jan 9, 2019

yamgent Jan 9, 2019

Markbind help: update output to match userguide #550

Markbind help: update output to match userguide #550

Conversation

JYL123 commented Jan 5, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Xenonym commented Jan 5, 2019

yamgent commented Jan 6, 2019

damithc commented Jan 6, 2019

Xenonym commented Jan 6, 2019

damithc commented Jan 6, 2019

JYL123 commented Jan 6, 2019

damithc commented Jan 6, 2019

JYL123 commented Jan 6, 2019

damithc commented Jan 6, 2019

yamgent commented Jan 7, 2019 • edited

build Command

...

yamgent left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yamgent commented Jan 8, 2019

JYL123 commented Jan 8, 2019

yamgent left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JYL123 commented Jan 5, 2019 •

edited

yamgent commented Jan 7, 2019 •

edited

`build` Command