i2van / hourglass

The simple countdown timer for Windows.
MIT License
27 stars 5 forks source link

add --validate-args option #7

Closed pivotiiii closed 3 months ago

pivotiiii commented 4 months ago

Hi, I added a new option called --validate-args on/off that checks if the provided CLI arguments are valid. If they are, it outputs the word true as well as the parsed TimerStart.ToString() value/s to stdout. If the arguments produce an error, false gets send to stdout. I also added the appropriate usage to the help dialog.

To test this in the command line (e.g. cmd.exe) you also need to add |MORE after the function call (e.g. hourglass.exe --validate-args on --title test until 3pm |MORE, because WinForms UI apps don't print their stdout to the console by default. If hourglass.exe is called via e.g. nodeJS with execFile() (for my use case), this is not necessary since it captures stdout by default. I use this for a plugin for Flow Launcher that sets a timer but needs to check if the supplied input is actually valid before presenting any options.

The portable build of hourglass builds with my changes, but the installer version throws an error about the installer program having duplicate symbols. This also happens on the develop branch without any changes. Feel free to ignore the updated workflow file.

I have not updated any of the version info files or anything else, just what was necessary to make this option work.

i2van commented 4 months ago

@pivotiiii Could you please provide real use case for this change; looks like it solves your specific problem mostly and might not be of any use for other users?

I could not accept this PR because it doesn't pass build as it should. I also could not ignore changes made to github actions file due to those changes are being the part of PR.

I think that it is better to create console application which does only options check instead of adding such an option to UI.

pivotiiii commented 4 months ago

The use case would be any program that acts as a front end for hourglass that accepts custom user input. This is definitely more of a developer option rather than one needed by regular users though, which is why it isn't included in any of the UI components but CLI only. Currently the only way to check if a set of arguments is valid is by running hourglass with the provided arguments and have the help dialog pop up. If the user input is to be checked for every entered letter or number, that would result in 10s of dialogs popping up when even one would already be horrible UX. The alternative would be as you said a new console application that checks the arguments and provides the same output as the --validate-args option right now. I feel like this would not be worth it though as any changes to hourglass would need to be added in and cross verified to work the same. Depending on the hourglass version compared to the console app there could be errors as well.

As for the building, the latest commit on your develop branch 31cdd0ba2face2764e98d68cfe120dc9b3e46ca4 without any changes does not build at the moment (here is the same commit in my fork not building). The last build on this repo seems to be 5 months ago, maybe something with the dependencies changed? The error is related to wixtoolset after all.

i2van commented 4 months ago

I fixed build. Please merge/rebase.

Have you considered this? Unfortunately it is just a console app emulation...

i2van commented 4 months ago
pivotiiii commented 4 months ago

I fixed build. Please merge/rebase.

Have you considered this? Unfortunately it is just a console app emulation...

I have looked at that previously, but all the options I found have drawbacks that I would consider worse than having to add |MORE if run from cmd. The AttachConsole version does not block the console, so if called in e.g. a batch script this could mix outputs. The other options have either a console window pop up on startup for a second or need too many changes to the code base.

done.

pivotiiii commented 4 months ago
  • Please provide a couple of command lines to test (with | MORE)

Hourglass.exe --validate-args on 3pm |MORE

true until 3 pm

Hourglass.exe --validate-args on --title TEST --always-on-top on 3pm |MORE

true until 3 pm

Hourglass.exe --validate-args on --multi-timers on 4m16s 3pm 4/7 |MORE

true 4 minutes 16 seconds until 3 pm 4 July

Hourglass.exe --validate-args on --wrong-switch on|MORE

false

Hourglass.exe --validate-args on --always-on-top wrong|MORE

false

Hourglass.exe --validate-args on 3km|MORE

false

i2van commented 4 months ago

Looks OK. I need to do some testing.

pivotiiii commented 4 months ago

OK so I just did some more tests myself, the output with |MORE fails if there is already a timer running.

pivotiiii commented 3 months ago

After some more testing, I came to the conclusion that there may just be no nice way to handle this problem. Once the stdout of the first instance of Hourglass is not outputting to the console anymore, i.e. after a timer has been started, any additional instances can't output anything. If you run the first timer with |MORE and run --validate-args in another window, you can see the output in the first console window.

I resorted to a console application that only checks the args, I was hesitant at first because I never worked with C# before this but adapting the Hourglass startup was pretty straightforward once I installed Visual Studio.

I can still see value in adding the --validate-args option to Hourglass, but if you don't want to, considering all the shortcomings of the current method, that is alright too.

i2van commented 3 months ago

Feel free to create PR with console app.

i2van commented 3 months ago

Closed due to technical difficulties.