In program help and better command error messaging #162

Closed
opened 2020-05-25 00:15:40 +00:00 by xfnw · 11 comments

For example if you try running the command add to add a bookmark, but use the wrong syntax it will make a very cryptic error message like Unknown action "ADD"

For example if you try running the command `add` to add a bookmark, but use the wrong syntax it will make a very cryptic error message like `Unknown action "ADD"`
xfnw changed title from Bookmarks cannot be created or deleted to Command syntax errors are unclear 2020-05-25 00:21:57 +00:00
Collaborator

Thanks @LickTheCheese for the report!

I can confirm this same experience. A specific example of the above would be a command to add the current page as a bookmark, forgetting to include the "title" argument:

:add .

The error message displayed is as mentioned above - Unknown action "ADD".

A similar message occurs for commands requiring a single argument if no argument is provided. So :delete says Unknown action "DELETE"

:set works in a similar way, but has another thing happening too. :set t says Unknown action "SET" but :set a says Found "A" (1), expected value

For most of this, it looks like the command interpreter is reading these commands as that of a different type. doCommandAs() handles ADD, and can return a more appropriate message if the number of arguments is not correct, but I don't think the parser will ever interpret "ADD" or "ADD ." in a way that gets routed to this function?

Also just a note this is not an issue new to 2.3.0, it also occurs in 2.2.0.

Thanks @LickTheCheese for the report! I can confirm this same experience. A specific example of the above would be a command to add the current page as a bookmark, forgetting to include the "title" argument: ``` :add . ``` The error message displayed is as mentioned above - `Unknown action "ADD"`. A similar message occurs for commands requiring a single argument if no argument is provided. So `:delete` says `Unknown action "DELETE"` `:set` works in a similar way, but has another thing happening too. `:set t` says `Unknown action "SET"` but `:set a` says `Found "A" (1), expected value` For most of this, it looks like the command interpreter is reading these commands as that of a different type. `doCommandAs()` handles ADD, and can return a more appropriate message if the number of arguments is not correct, but I don't think the parser will ever interpret "ADD" or "ADD ." in a way that gets routed to this function? Also just a note this is not an issue new to 2.3.0, it also occurs in 2.2.0.
Owner

You are correct @asdf :

The parser knows what the param count is for each type of action. As such, add is able to give error messages for anything that has two values after it. However, the parser does not know that add is even a command without those two params. It sees it as an unsupported action (which it is).

This is something that could get worked on, but will likely requre some level of rewrite to the command parser.

There is a way that we could work it that it might not require that though:

Create something like:

map[string]string{"add": "add [url | . | link id] [title...]"}

Then if a different command processor ends up with an "unknown" command, it can check the map for the first field in the passed string. If it exists in the map it can print/return the value associated with the key. If not, then it can print/return the error message from the parser.

This would certainly be the quickest way to get better error messaging, but has its limits; for instance, that it would just be able to respond with the correct syntax for a given command.

Does that seem like a reasonable fix? I'd prefer to not have to rewrite the parser if we do not have to, particularly given the availability of solid documentation for all of the commands via the man page.

Though that does bring up something else: I think we should install a text file to usr/share/[...] somewhere that running :help within bombadillo can pull up. It could basically be the man page, but without all the groff markup. But that is a separate issue.

You are correct @asdf : The parser knows what the param count is for each type of action. As such, `add` is able to give error messages for anything that has two values after it. However, the parser does not know that `add` is even a command without those two params. It sees it as an unsupported action (which it is). This is something that could get worked on, but will likely requre some level of rewrite to the command parser. There is a way that we could work it that it might not require that though: Create something like: ``` map[string]string{"add": "add [url | . | link id] [title...]"} ``` Then if a different command processor ends up with an "unknown" command, it can check the map for the first field in the passed string. If it exists in the map it can print/return the value associated with the key. If not, then it can print/return the error message from the parser. This would certainly be the quickest way to get better error messaging, but has its limits; for instance, that it would just be able to respond with the correct syntax for a given command. Does that seem like a reasonable fix? I'd prefer to not have to rewrite the parser if we do not have to, particularly given the availability of solid documentation for all of the commands via the man page. Though that does bring up something else: I think we should install a text file to `usr/share/[...]` somewhere that running `:help` within bombadillo can pull up. It could basically be the man page, but without all the groff markup. But that is a separate issue.
sloum added the
bug
documentation
enhancement
labels 2020-05-25 05:00:08 +00:00
Collaborator

Showing a syntax hint would be really useful here, possibly the ideal situation in my opinion.

Having the full documentation accessible within the application is a great idea too. It would be cool if :help command would take you to the relevant section of the document too.

Showing a syntax hint would be really useful here, possibly the ideal situation in my opinion. Having the full documentation accessible within the application is a great idea too. It would be cool if `:help command` would take you to the relevant section of the document too.
sloum changed title from Command syntax errors are unclear to In program help and better command error messaging 2020-06-13 23:03:17 +00:00
sloum self-assigned this 2020-06-13 23:03:27 +00:00
sloum added the
in progress
label 2020-06-13 23:03:38 +00:00
Owner

I have started work on this. I will have any command that does not map to a specific action (but the command itself is a valid command) show: Error. Command format: :help [section].

As a part of this story I will be leveraging the local protocol and our makefile. I am creating a directory of help files that can be referenced and laoded by the help command. They will get stored in /usr/local/share/bombadillo or the like. I'll post when I have a usable branch.

I have started work on this. I will have any command that does not map to a specific action (but the command itself is a valid command) show: `Error. Command format: :help [section]`. As a part of this story I will be leveraging the `local` protocol and our makefile. I am creating a directory of help files that can be referenced and laoded by the help command. They will get stored in `/usr/local/share/bombadillo` or the like. I'll post when I have a usable branch.
Collaborator

@sloum need a hand with this? I've had a look at updating this branch and implementing the improved error messages by itself. I can do a PR for this if it doesn't conflict with what you're working on?

@sloum need a hand with this? I've had a look at updating this branch and implementing the improved error messages by itself. I can do a PR for this if it doesn't conflict with what you're working on?
Owner

@asdf I'm not sure. The branch should be mostly done. I just havent gotten around to working on it in a bit. Though I have been having second thoughts about distributing a bunch of text files with the application and the resiliency of the system when those files cannot be found.

Have you installed off of the branch and tried out the help system? Does it seem like a good path to you?

If we are close to that branch being done I think finishing it up should be the priority since it includes the error message updates. If maybe I took a wrong turn with it or if it seems like it is still too far off, then maybe splitting the error message updates out would be a good idea... not sure. Let me know what you think.

@asdf I'm not sure. The branch should be mostly done. I just havent gotten around to working on it in a bit. Though I have been having second thoughts about distributing a bunch of text files with the application and the resiliency of the system when those files cannot be found. Have you installed off of the branch and tried out the help system? Does it seem like a good path to you? If we are close to that branch being done I think finishing it up should be the priority since it includes the error message updates. If maybe I took a wrong turn with it or if it seems like it is still too far off, then maybe splitting the error message updates out would be a good idea... not sure. Let me know what you think.
Collaborator

I did try it and was really impressed by what I saw. There was a compilation issue though, and the help commands weren't working after correcting it (just realising why that might be though). Plus the branch needs to be brought up to date with develop. It felt like it might be better to approach the improved error messages first, then the help commands afterwards. Then I kept getting curious and now the first part is nearly done lol.

Regarding doubt of "distributing a bunch of text files" I know what you mean, but don't know about another option. But maybe there are some additional things to try like using the makefile to substitute the correct file path during install, or maybe another fallback like getting them from the internet, or a message saying to read the man page because the files weren't found.

Anyway I'll submit what I have soon, if it's not the right way then we can just look at the original branch again.

I did try it and was really impressed by what I saw. There was a compilation issue though, and the help commands weren't working after correcting it (just realising why that might be though). Plus the branch needs to be brought up to date with develop. It felt like it might be better to approach the improved error messages first, then the help commands afterwards. Then I kept getting curious and now the first part is nearly done lol. Regarding doubt of "distributing a bunch of text files" I know what you mean, but don't know about another option. But maybe there are some additional things to try like using the makefile to substitute the correct file path during install, or maybe another fallback like getting them from the internet, or a message saying to read the man page because the files weren't found. Anyway I'll submit what I have soon, if it's not the right way then we can just look at the original branch again.
Owner

Awesome. Thanks for checking it out! I totally had not thought of the need to bring it inline with current develop.

Ok. Sounds like splitting out the error messages may be the way to go. From there we can see if the help files will make it into the next release or not (depending on how much is left and what options we can figure out to make it a bit more resilient).

The idea of using the makefile to insert a storage location is definitely something to look into.

Awesome. Thanks for checking it out! I totally had not thought of the need to bring it inline with current `develop`. Ok. Sounds like splitting out the error messages may be the way to go. From there we can see if the help files will make it into the next release or not (depending on how much is left and what options we can figure out to make it a bit more resilient). The idea of using the makefile to insert a storage location is definitely something to look into.
Collaborator

Just a quick note on the in-program help. In the last couple of days I have been looking at how other programs handle this, but haven't found an example where the program is at a similar level of maturity.

top has in-program help stored as strings in the source code, but this is to allow for internationalisation through gettext.

lynx uses local files to store help data, but has a myriad of configuration options surrounding it. For example, the path to the files is stored in a system configuration file (i.e. /etc/less) but this can be overridden by an environment variable. Help data can be stored as plaintext or compressed archives, chosen when you build the application. I think it also has multiple translations available.

I'm not that familiar with all the suckless tools, but couldn't see an example that featured in-program help. I'm guessing they just use man files and readmes?

A good comparison would help make this decision easier, but I think the original design of having lengthy user help pages within the application is a good idea. Local files is probably good enough too, we just have to cater for these situations:

  • a person clones the repo, runs make, then runs bombadillo. The help files are in ./help
  • a person clones the repo, runs make install, then runs bombadillo. The help files are in an installed location like /usr/local/share/doc/bombadillo
  • a person downloads a precompiled binary. Maybe the docs are in ./help?

There are a few solutions: the makefile can set the path conditionally, we might have failback to look for files in ./help or online. I'll keep thinking about it but would appreciate any feedback.

Just a quick note on the in-program help. In the last couple of days I have been looking at how other programs handle this, but haven't found an example where the program is at a similar level of maturity. *top* has in-program help stored as strings in the source code, but this is to allow for internationalisation through gettext. *lynx* uses local files to store help data, but has a myriad of configuration options surrounding it. For example, the path to the files is stored in a system configuration file (i.e. `/etc/less`) but this can be overridden by an environment variable. Help data can be stored as plaintext or compressed archives, chosen when you build the application. I think it also has multiple translations available. I'm not that familiar with all the suckless tools, but couldn't see an example that featured in-program help. I'm guessing they just use man files and readmes? A good comparison would help make this decision easier, but I think the original design of having lengthy user help pages within the application is a good idea. Local files is probably good enough too, we just have to cater for these situations: - a person clones the repo, runs make, then runs bombadillo. The help files are in `./help` - a person clones the repo, runs make install, then runs bombadillo. The help files are in an installed location like `/usr/local/share/doc/bombadillo` - a person downloads a precompiled binary. Maybe the docs are in `./help`? There are a few solutions: the makefile can set the path conditionally, we might have failback to look for files in `./help` or online. I'll keep thinking about it but would appreciate any feedback.
Owner

Another situation I have seen come up is people running go build or go install directly, rather than using the makefile. That ends up donking up bombadillo -v as well as the man page a desktop file (on linux).

Distributing the binary is a tricky one for the help files.

While I like the built in help pages (and put a lot of work into them), I have liked what we have currently in release2.3.3 with the :help [command name] structure just showing the syntax. I could see reworking the help pages into a nice manual built and managed with pandoc and distributed as a pdf or some such? I could also see trying to push through all these edge cases to get a decent and detailed help system. I suppose if we do so, we can try to detect the rpesence of help files and if they are not present, fall back to just printing a simple string of information to the bottom bar.

Another situation I have seen come up is people running `go build` or `go install` directly, rather than using the makefile. That ends up donking up `bombadillo -v` as well as the man page a desktop file (on linux). Distributing the binary is a tricky one for the help files. While I like the built in help pages (and put a lot of work into them), I have liked what we have currently in `release2.3.3` with the `:help [command name]` structure just showing the syntax. I could see reworking the help pages into a nice manual built and managed with pandoc and distributed as a pdf or some such? I could also see trying to push through all these edge cases to get a decent and detailed help system. I suppose if we do so, we can try to detect the rpesence of help files and if they are not present, fall back to just printing a simple string of information to the bottom bar.
Owner

Better help messaging in line with the issue report has made its way to master. I think we can and will continue to itterate on iproving help systems, but for the time being I hope that what has been added works well for folks. I am closing this issue as resolved since we got the error messaging fixed and added inline command help. That coupled with the manpage should handle most cases.

Better help messaging in line with the issue report has made its way to `master`. I think we can and will continue to itterate on iproving help systems, but for the time being I hope that what has been added works well for folks. I am closing this issue as resolved since we got the error messaging fixed and added inline command help. That coupled with the manpage should handle most cases.
sloum closed this issue 2021-01-22 06:01:59 +00:00
Sign in to join this conversation.
No Milestone
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: sloum/bombadillo#162
No description provided.