Fixed taglauncher crashing with certain apps installed #3705
Conversation
|
Do you think you could break that first commit up in one for the bugfix and one for the indentation change if it's not too much trouble? |
Yes, I'm going to do that now. (It's my first PR, so hopefully i wont break things... Otherwise, I'll open another PR and close this, which shouldn't be a problem, right? =) |
|
I believe I got it... Please tell me if something's still missing 😉 |
|
Tried it on my watch - seems to work well (I didn't try to reproduce the bug though). Thanks for the fix! Thinking again, lets not do the indentation change at all. I think it already follows the style guide pretty well (e.g. 2 spaces for indentation): https://www.espruino.com/Code+Style If you can drop the commit entirely that's neat. But reverting it is also fine :) Thanks for making your first contribution! 🚀 |
|
Done. I'm not sure whether my changes to the README are OK, because they are not really implicated by the PR's title... Should I remove them too? |
|
Nice - thanks for doing that! Merging now 🎉 |
|
The readme changes are good 👌 |
|
It's nice that you put the readme changes in separate commit/s when they didn't have to do with the other changes directly. |
If an app has no tags (e.g. Drained, in my case), it caused taglauncher to crash because of a missing check (and in turn running .split on undefined). This missing check was added and apps without the tag are now added to the Misc category automatically.
I also fixed some code indentation issues.
Real changes to app.js only from lines 51-61 in the old app.js (or ll. 75-90 in the new app.js).