Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed taglauncher crashing with certain apps installed #3705

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

BlueFox4
Copy link

@BlueFox4 BlueFox4 commented Dec 31, 2024

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).

@thyttan
Copy link
Collaborator

thyttan commented Dec 31, 2024

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?

@BlueFox4
Copy link
Author

BlueFox4 commented Jan 3, 2025

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? =)

@BlueFox4
Copy link
Author

BlueFox4 commented Jan 3, 2025

I believe I got it... Please tell me if something's still missing 😉

@thyttan
Copy link
Collaborator

thyttan commented Jan 3, 2025

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! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants