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.

Sign up for GitHub

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

Jump to bottom

Stop escaping ' on POT generation #86669

Merged
merged 1 commit into from
Jan 3, 2024
Merged

Stop escaping ' on POT generation #86669

merged 1 commit into from
Jan 3, 2024

Conversation

YeldhamDev
Copy link
Member

Escaping the ' character is unnecessary, and makes the resulting POT incompatible with translations tools such as Weblate.

@YeldhamDev YeldhamDev added this to the 4.3 milestone Dec 31, 2023
@YeldhamDev YeldhamDev requested a review from a team December 31, 2023 15:33
AThousandShips
AThousandShips reviewed Dec 31, 2023
View reviewed changes
Copy link
Member

@AThousandShips AThousandShips left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Is it only this escape that is incompatible? Also in what way is it incompatible, a link would be nice to avoid confusion 🙂

editor/pot_generator.cpp Outdated Show resolved Hide resolved
editor/pot_generator.cpp Outdated Show resolved Hide resolved
@YeldhamDev
Copy link
Member Author

@AThousandShips It will trigger errors in tools like Weblate, it happened to me with my own project. I don't know where this information is explicitly written however.

@KoBeWi
Copy link
Member

KoBeWi commented Jan 1, 2024

Makes me wonder whether the current implementation of c_escape() makes sense. You don't need to escape ' inside " string and vice-versa. Maybe it could be somehow accounted for.

KoBeWi
KoBeWi reviewed Jan 1, 2024
View reviewed changes
editor/pot_generator.cpp Outdated Show resolved Hide resolved
akien-mga
akien-mga reviewed Jan 2, 2024
View reviewed changes
editor/pot_generator.cpp Outdated Show resolved Hide resolved
@akien-mga akien-mga merged commit 34594a3 into godotengine:master Jan 3, 2024
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@akien-mga akien-mga added the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Jan 3, 2024
@YeldhamDev YeldhamDev deleted the there_is_no_escape branch January 3, 2024 13:32
@akien-mga
Copy link
Member

Cherry-picked for 4.2.2.

@akien-mga akien-mga removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AThousandShips AThousandShips AThousandShips left review comments

@Pheubel Pheubel Pheubel left review comments

@akien-mga akien-mga akien-mga approved these changes

@KoBeWi KoBeWi KoBeWi approved these changes

Assignees
No one assigned
Labels
bug topic:editor
Projects
None yet
Milestone
4.3
Development

Successfully merging this pull request may close these issues.
5 participants
@YeldhamDev @KoBeWi @akien-mga @Pheubel @AThousandShips