main: make cmd documentation coherent

[nonfungible-human]

This PR addresses some of the problems with the litcli --help message reported in Issue 591.

These changes affect the text displayed in the help messages and do not affect any functionality of the application. Everything else was left unchanged.

The changes are as follows:

1. The command categories were reorganized.
2. Descriptions were changed to be as consistent as possible in format, punctuation and capitalization. However, there were a few limitations as a result of the way the cli package works (see below). The result is a much more consistent appearance, but not perfectly consistent across all types of descriptions.
3. An attempt was made to make all line spacing consistent where possible.
4. All references to Terminal Web sessions were changed to Lightning Node Connect sessions.
5. Two sentences were copied from the Builder's Guide - Autofees section and appended to the existing help message for the privacy command in order to provide more clarity as requested in Issue 591. The more detailed description is available by running litcli privacy --help.
6. Multi-line quotes using the ` character were changed to regular double quotes, with newline characters added to preserve the paragraphs for the help messages that contain more than one paragraph of information.

Limitations of the cli package:

• The Description field in the Command struct used by the cli package is displayed differently based on whether the command has subcommands or not. If it has subcommands, the Description field is displayed as the short description. Otherwise, a separate section is displayed titled DESCRIPTION.
• The global help and version commands, as well as the help options for commands that have subcommands, seem to be provided by default by the cli package. These default commands come with default short descriptions. So the format of those descriptions was followed as closely as possible for consistency.

Formats of command and options descriptions:

• Short descriptions of global commands and options begin with upper case and do not end in a period.
• Short descriptions of subcommands begin with upper case and end in a period.
• Long descriptions of commands and subcommands begin with upper case and end in a period.
• Options for commands that have subcommands are all lower case.
• Options for commands that do not have subcommands begin with upper case and end in a period.

[Liongrass]

Awesome! I should be able to look at that soon!

[Liongrass]

Hi! Thank you very much for that contribution! I looked through it and also compiled from your pull request, looks good!
If you find the time, maybe you could also put the first character of the GLOBAL OPTIONS to upper case?

In LND we aren't consistent with whether sentences end with a . or not, I think for LiT we can always let them end with a sentence.

I'll let @ViktorTigerstrom comment on whether he wants to surface litcli actions in the man page.

[nonfungible-human]

Yes, I can change the global options to start with an upper case letter. Should I wait to find out about the Actions category, or just make that change and create a new pull request?

[ViktorTigerstrom]

Hi @nonfungible-human! First of all, thanks a lot for the contribution, it's much appreciated 🙏🔥!

I'll start by commenting regarding moving actions to it's separate category, and then I'll provide a detailed review of small nits once my "categories" feedback has been addressed.

I've discussed a bit internally, and I think it's starting to get a bit confusing that we have lot's of categories with just 1 entry. Categories as a concept is then starting loose it's meaning.

Instead, I suggest that we change the categories a bit, so that they become groupings with relevant commands underneath.
I suggest the following structure (but also, do feel free to suggest your own version of course, if you think you have a better suggestion!):

Accounts:
  accounts

Firewall:
  actions
  privacy

LiT:
  bakesupermacaroon
  helper
  getinfo
  stop
  status

LNC:
  autopilot
  sessions

Lastly, this PR now has a separate commit that merges with the master branch. That's not our preferred commit structure. What you'd instead like to do, is to your rebase this branch on top of the latest upstream master branch, so that this PR only contains the individual commit with the change. You'll need to drop your merge commit when doing so. Then force push your branch, and this PR will contain the latest changes without the merge commit. I can guide you of how to do this I'd you like :).

[nonfungible-human]

@ViktorTigerstrom I think your suggestion for the categories is a good idea and can definitely make that change as well.

This is my first pull request for this project, so I'm not familiar with the process yet and am not sure how to rebase this branch on top of the latest upstream master branch.

Right now, on my forked repository, branch issue591 is telling me This branch is 2 commits ahead of lightninglabs:master. which is what I would expect.

On my local system, this is what I see when I run git status or try to do a git pull.

$ git status
On branch issue591
Your branch and 'origin/issue591' have diverged,
and have 1 and 3 different commits each, respectively.
  (use "git pull" to merge the remote branch into yours)

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   go.mod
	modified:   go.sum

no changes added to commit (use "git add" and/or "git commit -a")

$ git pull
error: Your local changes to the following files would be overwritten by merge:
	go.mod
	go.sum
Please commit your changes or stash them before you merge.
Aborting

[ViktorTigerstrom]

@ViktorTigerstrom I think your suggestion for the categories is a good idea and can definitely make that change as well.

Great, thanks!

Ok, I'll try to help out regarding the rebase issues! I'd also recommend reading the Bitcoin Core contributors guideline in regards how to handle the rebase flow, as it explains the default process: contributors.

For your specific situation right now, I think running the following would commands in these steps should help:

1. Make sure your local issue591 branch is checked out.
2. git rebase -i origin/issue591
3. git rebase -i origin/master
   When the interactive rebase interface appears, you need to mark the commit that merges this branch with master with drop. If you haven't configured your git settings, that's usually done by navigating with the arrows on your keyboard until the commit is marked, and then pressing the button d. Then press w to close and save, and press y to confirm.
4. Do all the edits you need (possibly in multiple commits, but it's also okay to squash all these changes into 1 commit). Once you're ready, run git push --force, which will update this PR with your changes.

Let me know if that helps!

[nonfungible-human]

@ViktorTigerstrom I ran the rebase commands and I'm pretty sure it worked correctly. I made the changes and pushed them out to my forked repo. So I think everything should be ready to go now.

I ran the rebase command again after reading the info you posted. Now I believe my commits are all combined into one.

[Liongrass]

Looks good! Thank you!

[ViktorTigerstrom]

Thanks a lot for implementing the new category structure, it looks great!

Unfortunately this PR still includes the merge commit with master, as well as a dependabot bump commit which shouldn't be part of this PR. So currently the PR includes 3 commits, but should only contain the one commit that contains your changes.
So you'd need to run the rebase command again to solve that, you'll know that it was successful once the PR only contains the single commit containing your changes only :).

Try once more to run:
git rebase -i origin/master

When you do that, you'll see the interactive git interface that let's you choose which commits should be kept in the branch, and which should be dropped. Ensure that all commits except the commit containing your changes is marked as drop, and then press w to end the interactive rebase.

I've also added some small feedback with some "Usage" fields for flags that aren't following the style you've decided to implement. It'd be great if you could correct those :)!

Finally, one last thing I definitely think we also should fix with this PR, is that we have inconsistent string types for different "Description" sections throughout the help docs.
Some "Description" sections uses single quotes i.e. the ` char, and some "Description" sections uses double quotes i.e. the " char. This leads to an inconsistency that we'll sometimes have leading newline, and sometimes have trailing new lines. It would be awesome if you could fix that as well!

[nonfungible-human]

@ViktorTigerstrom It looks like the rebase worked correctly this time. After pushing to my fork, the branch is ahead of master by 1 commit. I also made the two changes you pointed out in your last comment.

[ViktorTigerstrom]

Awesome thanks! This is almost good to go now 🎉🚀!

It looks like the rebase worked correctly this time. After pushing to my fork, the branch is ahead of master by 1 commit.

Yes great, it looks like the rebase worked correctly this time, thanks!

I also made the two changes you pointed out in your last comment.

Perfect thanks! Would you also be able to fix the issue with the "Description" fields I commented about above? I.e. that we have inconsistent string types for different "Description" sections throughout the help docs.
We should change so that all "Description" fields uses double quotes for the string type. I've added one example of what needs to be changed in my review below, but please note that there are multiple "Description" fields throughout the docs, where this needs to be addressed.

Lastly, could you please update the commit title of your commit message to align with our contribution guidelines for commit messages LND contribution guidelines?
If you'd like, here's a suggestion for the commit message title that you could use if you'd like:
main: make cmd documentation coherent

Please feel free suggest another commit message title though! Just make sure that it starts with "main:" and is <= than 50 chars, doesn't start with an upper case letter, nor ends with a period.

[nonfungible-human]

@ViktorTigerstrom I've made the changes you mentioned. For the strings that changed to double quotes, some of those messages were made up of multiple paragraphs of text, so I added newline characters to separate the paragraphs and spaces to preserve the indentation that was there before.

[ViktorTigerstrom]

Great thanks a lot! Awesome job 🚀🔥!!

tACK LGTM after some final nits have been addressed.

1. Thanks a lot updating the string type of the various description fields! Unfortunately though, one side effect of the made changes is that there are now multiple occasions where the code doesn't adhere to our code formatting rules, as the code should wrap at 80 chars with tabs treated as 8 chars. I know it's a bit annoying to fix these small things. Would you be able to update so that the "Description" fields you changed are wrapped at 80 chars?

2. Thanks for updating to title to "main: make cmd documentation coherent". Though just to clarify, you need to also update the title actual commit message of the 1 commit this PR includes, and not just this PR title. Currently the commit message title is: "An improvement to the litcli help messages.". Once you've made the above changes, if you run the following command, you should be able to change the commit message:
   git commit --amend
   You can also add the -m after that to set the actual commit message if that's what you prefer.

Once this has been addressed, this should be good to go 🚀🔥!!

[nonfungible-human]

@ViktorTigerstrom I'm pretty sure all the lines are within the correct length now. I've also changed the commit message. I think everything should be ready to go.

[Liongrass]

Hi! Congrats on your first commit! I have looked through all your changes and find them consistent, legible and in accordance with our style and writing.
I've also run the changes locally and looked at the new manuals and help pages. Big noticeable improvements, thank you! ACK!
There are still some instances where lit is written in small caps, but I don't see this as a required change for this PR.

[ViktorTigerstrom]

Awesome! Thanks a lot for the review @Liongrass.

@nonfungible-human, great job, and congratulations on your first contribution to this repo 🙏!! There's been a small change to the master branch since your last push. Would you be able to update your local master branch to the latest version, and then run git rebase -i master when having this branch checked out and then force push this once more? We can then merge this PR after that's been addressed 🔥!

[nonfungible-human]

@ViktorTigerstrom Okay, I think that worked. I pulled master into my fork, and then to my local repo, did the rebase and pushed it back out to my fork. But it still tells me I am 1 commit ahead and 2 commits behind.

[ViktorTigerstrom]

But it still tells me I am 1 commit ahead and 2 commits behind.

Yes unfortunately your branch is still not based on master, so I can't merge it. I think that what happened is likely that your local master branch isn't synced with the upstream repo, and therefore when you rebase on your local branch you'll still be behind.
So let's first sync your local master branch with the upstream repo. One easy way you can do that is the following:

1. Navigate to the master branch on your forked repo on github:
   https://github.com/nonfungible-human/lightning-terminal/tree/master
2. At the right part of the section that says "This branch is 2 commits behind lightninglabs:master.", there should be a "Sync fork" dropdown. If you click that dropdown, there'll be a green "Update branch" button. Press that button and your upstream master branch of your forked repo will be synced.
3. In your local repo, checkout the local master branch, and then run git pull. Now your local master branch will be up to date with upstream master branch as well.
4. Now finally, checkout your issue591 branch and rebase it on master with git rebase master.
5. Force push the issue591 branch and this PR should be based on the master branch 🚀!

[nonfungible-human]

@ViktorTigerstrom Okay, I was doing it a little differently yesterday. It now tells me my fork is just 1 commit ahead of master. So I think it is finally ready to go.