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

Better menu item injection #45

Closed pgaskin closed 4 years ago

pgaskin commented 4 years ago

closes #43

pgaskin commented 4 years ago

So far, this works (and it's stable), but the code needs a lot of cleanup (it's still a mess from when I was trying different techniques), which I'll be doing tomorrow or the next day. You can still leave comments now if you have any questions or suggestions.

I'm probably going to see if I can do something about the fact that this is slightly more fragile than the old method. It should still work on all firmware version, though (but I'd appreciate it if anyone can test this on older fw versions).

pgaskin commented 4 years ago

I think this is as clean as it's going to get for now.

pgaskin commented 4 years ago

And a quick note about where this is more fragile than the old method:

On the whole, these additional points of breakage are irrelevant since they're unlikely to change without the symbols also changing (which will be caught by NM). This probably won't happen unless nickel is redesigned.


P.S. And, the points where the old method is fragile (which also apply to this one) is:


In other words, NM won't be much more fragile than it was before, and it wasn't that fragile to begin with (there's only one or two places it could segfault, and the possibilities for silent errors are more likely to cause a non-silent one first).

shermp commented 4 years ago

I'm afraid I can't offer much of a review (I'm still a bumbling idiot when it comes to Qt and C++), but I couldn't see anything obviously wrong, and my Kobo didn't break.

NiLuJe commented 4 years ago

Yup, nothing imploded on my end either ;).

pgaskin commented 4 years ago

OK, I'll merge this once I implement #25 and release v0.1.2. Then, I'll work on #42 (which should be pretty simple now).