search

labelle-org / labelle

Label printing software
Apache License 2.0
27 stars 4 forks source link

Enable listing of devices without list-devices subcommand #27

Closed maresb closed 3 months ago

maresb commented 4 months ago

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 commented 4 months ago

Ugh, I thought that I had rebased this out of https://github.com/labelle-org/labelle/pull/22/commits/be39218fc63c153a1ef8fc213ddc8cb9eb3c3db4. 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 commented 4 months ago

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 commented 4 months ago

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

maresb commented 4 months ago

main has been fixed in #30.

maresb commented 3 months ago

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