-
-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[feat] use clap Derive API to accept command line arguments #247
Conversation
Hi, due to health reasons I had to take a small break from my responsibilities. CC: @zubairmh Can someone just test this pr? This seems good to go for me. I'll merge if it's fine. |
@zubairmh Any updates on this? I tested this from my end and everything looked fine to me. |
Hey @InnocentZero! I just tried testing out the PR and it seems that the config struct has some small errors that cause the daemon to panic. The exact error is in the comment I left if that helps. The arg parser tried to make an option from the first char in the option, so in this case, in both cases of config (-c) / cooldown(-c) as well as debug(-d) / devices(-d), it sort of conflicts. Removing the |
Oh that was my bad. I did not thoroughly check the daemon and committed the changes. Should have paid attention! Thanks for pointing it out @newtoallofthis123 ! |
@Shinyzenith alternatively we can also use |
CC: @newtoallofthis123 How do you feel about this? |
Usually I did just test the latest commit, everything seems to work fine now :) PS: I think you might've accidentally pushed the |
Glad to have it sorted, and sorry for the mixup 😅
Oh I just rebased my local branch based on upstream to prevent merge conflicts. I've revised the commits. |
@Shinyzenith can you go through this once? Thanks. |
Hi |
Same goes for |
This is for As for I'll change it to the previous defaults. |
Hi, Apologies forn the misunderstanding. I am a maintainer so it should've gone under a review cycle, but it's fine now that it's reverted.😊 |
Sorry, I hadn't noticed the flags being used currently 😅 |
Ready to merge this, can you just rebase it onto the current main to resolve the conflicts? If not, I'll do it myself |
Hi conflict still exists and now the commits are repeated. |
This patch removes the usage of clap::Command and arg macros, and bumps the library version to use the new derive API which is the recommended method upstream. Signed-off-by: innocentzero <[email protected]>
This patch follows the previous one to update the method of taking arguments for swhks. The derive API is the recommended method upstream. Signed-off-by: innocentzero <[email protected]>
Signed-off-by: innocentzero <[email protected]>
This patch fixes the conflicting short options in swhkd and implements alternative short option forms. Signed-off-by: innocentzero <[email protected]>
This change is meant to make the changes backwards compatible for the user. Signed-off-by: innocentzero <[email protected]>
@Shinyzenith can you check now, I actually messed up initially while rebasing. Hope it is fixed. |
Signed-off-by: innocentzero <[email protected]>
Thanks for the patch set! |
My absolute pleasure 😄 Hope to contribute more significant patches in the future! |
This patch removes the usage of clap::Command and arg macros, and bumps
the library version to use the new derive API which is the recommended
method upstream.