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

[i18nIgnore] Fix highlighted code errors #5817

Merged
merged 3 commits into from
Dec 17, 2023
Merged

Conversation

itsmatteomanf
Copy link
Contributor

Fixes a small issue in rows selected in a code block, off-by-one errors.

Copy link

vercel bot commented Dec 17, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview Dec 17, 2023 9:54pm

@itsmatteomanf
Copy link
Contributor Author

Not sure if I should probably update translations, if they exist...

Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

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

Looks good to me! Usually we suggest avoiding updating multiple languages at once, but for a small fix like this, if you can fix all other languages where this is present, that would also be acceptable (and we’d stick the i18nIgnore keyword into the commit message to avoid triggering translation status changes).

@github-actions github-actions bot added the i18n Anything to do with internationalization & translation efforts - ask @YanThomas for help! label Dec 17, 2023
@itsmatteomanf
Copy link
Contributor Author

itsmatteomanf commented Dec 17, 2023

@delucis done all other languages, the issue was that some space was added (or removed) and the numbers didn't align. Found a language which had numbers wrong even more ahah

Do check that the commit message is correct with the correct usage of the keyword.

Usually we suggest avoiding updating multiple languages at once, but for a small fix like this, if you can fix all other languages where this is present, that would also be acceptable (and we’d stick the i18nIgnore keyword into the commit message to avoid triggering translation status changes).

I asked because of this, I know the policy, which makes sense, but it was such a small fix...

@delucis delucis changed the title Fix highlighted row errors [i18nIgnore] Fix highlighted code errors Dec 17, 2023
Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

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

Thanks @itsmatteomanf! Noticed one of these that looks off.

src/content/docs/es/core-concepts/astro-components.mdx Outdated Show resolved Hide resolved
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
@itsmatteomanf
Copy link
Contributor Author

Thanks @itsmatteomanf! Noticed one of these that looks off.

I accepted it... but locally looks like this.

VSCode doesn't handle it, as it doesn't align with what it has locally, as you are asking to change it how it already is ahah

image

@delucis
Copy link
Member

delucis commented Dec 17, 2023

I see the comment still there in your screenshot (second line here):

Two lines both of which include the code sample title, first as a title attribute, then as a code comment

So I think all’s well — let’s see how the deploy preview looks after that commit.

Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

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

Yup, looks correct now! 🙌

code block with the expected lines highlighted

@delucis delucis merged commit 00340b4 into withastro:main Dec 17, 2023
6 checks passed
@itsmatteomanf itsmatteomanf deleted the patch-2 branch December 17, 2023 21:56
ematipico pushed a commit that referenced this pull request Jan 26, 2024
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
i18n Anything to do with internationalization & translation efforts - ask @YanThomas for help!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants