Skip to content

Conversation

maresb
Copy link
Contributor

@maresb maresb commented Apr 28, 2024

In terms of keeping the CLI simple, I think avoiding subcommands may be a worthy goal. I find it really elegant when labelle --help prints a complete summary without needing to explore a tree of help of subcommands.

Thus I'm offering this alternative proposal. Introducing a special --device list case. It's a bit of a hack, but I think it's a very practical hack: nobody will ever want to filter by the string "list". Also, when the user is trying to figure out what they can filter on, they'll be reading the annotation for --device, and the info is provided there.

It seems to me like this sacrifices some mildly ugly code to provide a more focused user interface. Thoughts?

@maresb maresb force-pushed the remove-device-list-subcommand branch from 7578640 to 6801f3a Compare April 28, 2024 13:17
@maresb
Copy link
Contributor Author

maresb commented Apr 28, 2024

Ugh, I thought that I had rebased this out of be39218. Sorry!!! That PR #22 promised type annotations, not an interface change.

This PR currently implements some very minor code cleanup from that commit. In case people don't like this idea we can revert the interface change before the next release.

@tomers
Copy link
Contributor

tomers commented Apr 28, 2024

I really don't like the idea of --device list. I don't think it contributes to the user interface. It does not result in a more focused user interface TMO.
Moreover, I think we'll need some more commands in the future, such as labelle status to show the status of the current/selected label printer. I just read the technical specification of LabelWriter 550, and there's many interesting fields that I would like to expose in that command, but are of no interest to the user in the regular flow.

@maresb
Copy link
Contributor Author

maresb commented Apr 28, 2024

Ok, I will fix main right now. Let's discuss the interface later in more depth.

maresb added a commit to maresb/labelle that referenced this pull request Apr 28, 2024
maresb added a commit to maresb/labelle that referenced this pull request Apr 28, 2024
@maresb maresb force-pushed the remove-device-list-subcommand branch 2 times, most recently from 5fe2203 to 5b1d401 Compare April 28, 2024 14:27
@maresb
Copy link
Contributor Author

maresb commented Apr 28, 2024

main has been fixed in #30.

@maresb maresb force-pushed the remove-device-list-subcommand branch from 5b1d401 to c59a4c9 Compare May 4, 2024 14:18
@maresb
Copy link
Contributor Author

maresb commented May 5, 2024

I think #28 now contains a viable workaround so that this is no longer necessary.

@maresb maresb closed this May 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants