Skip to content

Conversation

@ids1024
Copy link
Contributor

@ids1024 ids1024 commented May 12, 2021

No description provided.

@ids1024
Copy link
Contributor Author

ids1024 commented May 12, 2021

Previously, linuxdeploy raised an error if the desktop file contained a line with a - in the key and a locale, since the locale checking code didn't allow -, and was applied to entryName. This seems to have broken AppImage builds for Popsicle.

@ids1024
Copy link
Contributor Author

ids1024 commented May 12, 2021

Oh, I see @mmstick opened an issue for this.

This should fix #5.

(c >= 'a' && c <= 'z') ||
(c == '_') || (c == '@')
(c == '_') || (c == '@') ||
(c == '[') || (c == ']')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This permits the use of [ characters inside the locale part. I think you should remove this line, and iterate over the inner part only.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, no, this is handled elsewhere already. However, I find it confusing to permit these here. I am going to update your branch to iterate only over the inner part of the string.

@TheAssassin
Copy link
Member

TheAssassin commented May 13, 2021

We need to add a test case to make sure this won't happen again in the future.

@TheAssassin
Copy link
Member

Seems like the existing check is already pretty broken. We have a test that is failing already on a locale like de_DE.UTF-8 (which, according to https://specifications.freedesktop.org/desktop-entry-spec/latest/ar01s05.html, should be valid). I'll fix that as well.

@TheAssassin TheAssassin merged commit 7075b80 into linuxdeploy:master May 13, 2021
@ids1024 ids1024 deleted the locale branch May 13, 2021 22:33
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