search

pgaskin / NickelMenu

The easiest way to launch scripts, change settings, and run actions on Kobo e-readers.
https://pgaskin.net/NickelMenu
MIT License
512 stars 25 forks source link

Switch to new method for nickel_extras:web_browser and implement url/css options #50

Closed pgaskin closed 4 years ago

pgaskin commented 4 years ago

closes #49

pgaskin commented 4 years ago

I've tested this myself with:

menu_item : main : browser1 : nickel_extras : web_browser
menu_item : main : browser2 : nickel_extras : web_browser :         https://pgaskin.net
menu_item : main : browser3 : nickel_extras : web_browser :         https://pgaskin.net  * { background: gray !important }
menu_item : main : browser4 : nickel_extras : web_browser : modal
menu_item : main : browser5 : nickel_extras : web_browser : modal : https://pgaskin.net
menu_item : main : browser6 : nickel_extras : web_browser : modal : https://pgaskin.net  * { background: gray !important }
shermp commented 4 years ago

Quick thoughts:

I can't say I'm fond of the fact there is only whitespace between the URL and css, but I can understand why it's so, and not sure if you could handle it better, so...

Feeding in some junk string in place of CSS didn't break it, so that's nice.

pgaskin commented 4 years ago

I can't say I'm fond of the fact there is only whitespace between the URL and css, but I can understand why it's so, and not sure if you could handle it better, so...

Yeah. I'm pretty sure I'm going to get at least one person thinking there should be a colon there instead and complaining that their config doesn't work...

shermp commented 4 years ago

Yeah. I'm pretty sure I'm going to get at least one person thinking there should be a colon there instead and complaining that their config doesn't work...

Knowing what you know now, would you choose a different delimiter, or even a format like yaml/json/toml/ini if you were starting afresh?

pgaskin commented 4 years ago

Knowing what you know now, would you choose a different delimiter, or even a format like yaml/json/toml/ini if you were starting afresh?

Almost definitely not. I've had less (none, actually) complaints of configuration errors with my custom format compared to other general-purpose languages (remember quoting in the old patch system and indentation in the new one, plus some non-techy people not understanding sections in INI files for other mods?). In addition, it's significantly easier to validate it and give meaningful error messages. Also, it's a lot more concise then anything else could be (e.g. the only one which would be able to handle chains sort-of-concisely is TOML).

I still think the delimiter was a good choice overall, as this is the only time it has been unsuitable (because we need more than one freeform unrestricted option). Note that in the event I ever need more config options (possibly with more escaping), I'd add a option:<key>:<value> type to the config file which would add arbitrary options to the most recently added config item.

For lack of a better place to put it, here's a branch with all the recent PRs merged.