hyprland-community / Hyprkeys

A simple, scriptable keybind retrieval utility for Hyprland [maintainer=none]
GNU General Public License v3.0
120 stars 6 forks source link

Large refactor and some todo list items completed #4

Closed abs3ntdev closed 1 year ago

abs3ntdev commented 1 year ago

I spent some time refactoring this for personal use.

JSON output has been added output to passable file command line argument improvements (getopt) handles and displays comments in JSON and markdown output handles pipes in markdown output updated markdown output to not have the "bind* =" part added Makefile for install config file parsing to read all variable categories and subcategories proper go tests and removed the --test flag, outputs all test results to test/ directory and probably something i cant remember

if you want anything done differently let me know I am happy to work on it.

NotAShelf commented 1 year ago

Thanks for your interest! I currently do not have the time, but I'll review this as soon as possible.

flick0 commented 1 year ago

theres an empty main file at root, otherwise lgtm

and about the util/parser/parser.go and util/properties/properties.go files, as far as i can see you arent using them anywhere so you shud be good to delete them, the parser.go file is very messy anyway and theres a better alternative to just use hyprland socket instead of reading config file and parsing manually. what do you think @NotAShelf

abs3ntdev commented 1 year ago

should be fixed now. removed the main and unused utils. since we are already reading the file for the binds(cant get that from socket unless its hiding from me somewhere) its barely more work to read the config values as well. as for using the socket is there a way to get all options without batch running a getopt for everything? i might have missed it or its undocumented. if you can then that would be easier since its already output in json.

flick0 commented 1 year ago

as for using the socket is there a way to get all options without batch running a getopt for everything? i might have missed it or its undocumented.

iirc the socket allows you to send a batch of cmnds at once using --batch

abs3ntdev commented 1 year ago

yeah but i mean like how would you get all the settings via socket without knowing the names of every single specified setting before hand? reading from the file you just need to scan for the category names but via socket (unless im missing someting) you have to manually call every genera:thing general:thing2 general:thing3 right?

flick0 commented 1 year ago

i dont think there is anything to get all the settings at once without knowing each option thru socket, i had to do something like this for hyprland-py and i ended up scraping hyprland wiki to get all the config options then run getoption for all of them

abs3ntdev commented 1 year ago

Yeah but even then you miss any device:name custom config options set by the user. I think just parsing the file is fine unless they eventually let you query for all settings from the socket. Up to you guys though. The new reader is pretty easy to add new categories/subcategories to tho.

NotAShelf commented 1 year ago

I prefer reading from a static config file (i.e. ~/.config/hypr/hyprland.conf). We can attempt to use the socket1 for receiving config values, but I think it's easier to alter the codebase to accommodate newly added config options.

NotAShelf commented 1 year ago

With the addition of hyprctl binds, we can probably streamline the code even further. Should be as simple as parsing the output of hyprctl binds -j and reading the json output. The downside is, however, that we cannot cherry pick comments and add them to the markdown.

abs3ntdev commented 1 year ago

if you want to have the config file conversion eventually then i would leave the file reader writer and build on it. but we can use hyprctl binds to get binds set during runtime

abs3ntdev commented 1 year ago

i added the hyprctl reading and converting the modmask number. if your bindings have double quotes in it hyprctl doesnt validate the json before returning it so you get errors. i just changed all my double quotes to single and it works fine.

NotAShelf commented 1 year ago

Could you try if it works after the fix vaxry implemented? I am inclined to believe this is ready for merging (although I was intending to make parsing comments a command line flag) if it works on the latest version of Hyprland.

Side note, if you could record a quick demo gif (using a tool such as vhs, perhaps) to put in the readme I would appreciate that.

abs3ntdev commented 1 year ago

yeah it works after the recent fix. i can make comments a command line argument rq and record a video sure.

NotAShelf commented 1 year ago

Thanks!

abs3ntdev commented 1 year ago

okay i added the flag and video and picture to readme

NotAShelf commented 1 year ago

The --auto-start flag seems to be outputting the entire config, could you see if it occurs on your end too

abs3ntdev commented 1 year ago

so right now the behavior is binds are always output then you can add additional things by adding flags. so --auto-start prints binds + execs --variables prints binds + variables. would you rather default output nothing and have a --binds flag to print the binds.

abs3ntdev commented 1 year ago

i added a --binds flag it should work more how you were expecting now

NotAShelf commented 1 year ago

Ah, I wasn't trying to tell you to change the behaviour as I was only confused, but thanks regardless.

LGTM! I will be merging this unless there is anything else you would like to add?

abs3ntdev commented 1 year ago

nah seems fine for me. and it makes more sense to work with flags so idm.