IITC-CE / ingress-intel-total-conversion

intel.ingress.com total conversion user script with some new features. Should allow easier extension of the intel map.
https://iitc.app
ISC License
285 stars 110 forks source link

replace pluginBkmrksEdit hook with various different hooks #353

Open Aradiv opened 4 years ago

Aradiv commented 4 years ago

This hook gets triggered all the time with various different targets making every implementation that check this hook having a huge switch/case or if to cover the actions.

Available actions

also a lot of the actions have different targets

and some of the actions are only valid with one of the targets making the target pretty much obsolete.

reset/import => always all open/close => always folder add/remove => portal/map/folder

My Suggestion would be ro replace this 1 hook with 6 seprate hooks so hook functions just need to run for their respective case and not for everything (eg most things that run on that hook probably don't care about open/close folders) also handling reset/import is a completly different thing from add/remove an entry in the bookmarks

johnd0e commented 4 years ago

My Suggestion would be ro replace this 1 hook with 6 seprate hooks

Shouldn't we keep old hook too, for compatibility?

Aradiv commented 4 years ago

yes but we should mark it deprecated an warn people when they are using it.

generally we need a deprecation ruling on how and when we want to break BC and remove deprecated stuff to get IITC evolved to a more modern system.