Skip to content

refactor(l10n): macro gen and verify all strings exist #179

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Kyza
Copy link

@Kyza Kyza commented May 21, 2025

Draft for #165 and (might) fix #74.

Implements basic macro generation for the localizations and verifies that all languages have all possible strings with a test.

It looks like the only way to generate a #[cfg(feature = "name")] is through a proc macro which would require a separate crate. Considering Edit's probably very intentional lack of dependencies and current structure I decided to leave that out at least for now.

I haven't formatted the macro yet because that work is better suited for when this is finished.

It might be worth adding a language switcher to the menubar as well if data storage is viable.

@Kyza
Copy link
Author

Kyza commented May 21, 2025

@microsoft-github-policy-service agree

@Kyza
Copy link
Author

Kyza commented May 21, 2025

I didn't realize #85 existed. I can rebase this off of those changes once it gets merged.

pt_br: "Ctrl",
ru: "Ctrl",
zh_hans: "Ctrl",
zh_hant: "Ctrl",
Copy link
Member

Choose a reason for hiding this comment

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

Oooooh... That's neat. 😲 Way neater than what I had in mind!

I think may be beneficial to keep as much outside the macro as possible though. Isn't editing macros still a bit of a pain in most IDEs?

Copy link
Author

Choose a reason for hiding this comment

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

It is pretty annoying. You end up losing suggestions. I just worked on moving some code outside of the macro.

@Kyza
Copy link
Author

Kyza commented May 22, 2025

Admittedly I'm not that great at Git. I tried rebasing on the latest commit from main, but I'm not entirely sure if I did it right.

Edit: Definitely did not do that right. I ended up force pushing. lol

)+
]
}
pub const fn name(&self) -> &'static str {
Copy link
Author

Choose a reason for hiding this comment

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

This would be for using the name of the language as a string in the UI. For example if a language switcher is added.

@Kyza Kyza marked this pull request as ready for review May 22, 2025 03:17
@lhecker
Copy link
Member

lhecker commented May 22, 2025

I may need to come back to this PR later, as I want to focus on bug fixes first. There's quite a few of them: https://github.com/microsoft/edit/issues?q=is%3Aissue%20state%3Aopen%20label%3AI-bug
I don't plan to merge any of the translation PRs until after this PR (or some variant of this PR) has merged though, so hopefully there should be no further merge conflicts. 😊 Thank you for your patience!

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.

Localization is broken
2 participants