Several improvements #14
Conversation
| echo "usage: $(basename $0) [options] path-to-file [path-to-file]..." | ||
| echo " -h --help print this usage information and exit" | ||
| echo " -q --quiet don't print progress information" | ||
| echo " -r --toggle-restart toggle whether to restart the tablet (default is given by RESTART_XOCHITL_DEFAULT)" |
There was a problem hiding this comment.
"toggle-restart" and "restart the tablet" could be confusing/misleading, since systemctl restart xochitl is just restarting the UI service and not rebooting the tablet. for many people "restarting" a device is a reboot or power cycle.
There was a problem hiding this comment.
Fair point. What do you suggest instead? Saying "restart the tablet's UI" would be accurate but unenlightening, as it does not explain the purpose behind the restart. One could say "refresh the document database on the tablet" but does not explain that the whole UI will refresh.
There was a problem hiding this comment.
i'm just doing a drive-by comment since it's really up to @adaerr -- but FWIW i'd say that rather be technically-correct (but possibly confusing), be user-focused and use terminology that explains the effects visible to the user rather than how it's accomplished. so my suggestion is something like --make-visible or --show-docs-now or something along those lines, and the extended help comment would explain by saying "make the transferred document visible in the UI now (rather than after next reboot / xfer with --make-visible)" or something similar.
There was a problem hiding this comment.
Thanks for the suggestions. I feel that it would still be quite weird to have -r short options to expand to --toggle-make-visible, and the latter is quite a mouthful. And we must have -r be named -r and act as a toggle, or we break backward-compatibility. I leave to @adaerr to make the final call.
| } | ||
|
|
||
|
|
||
| # Parse arguments |
There was a problem hiding this comment.
would using getopt(1) be easier? or are there linux/osx compatibility issues?
There was a problem hiding this comment.
I was not aware of getopt(1). Looking at the docs now, it indeed could have saved a little bit of work. I will not attempt change to getopt(1), as it does not seem to offer huge advantages. However, I will keep it in mind when I write argument-parsing next time.
One thing that I would have preferred is to have the "-r|--restart" option to force the restart, as opposed to the current toggle behavior. However, I kept the old behavior for backwards-compatibility.