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

Set code languages for various files (part 1) #28145

Merged
merged 10 commits into from
Aug 2, 2023
Merged

Set code languages for various files (part 1) #28145

merged 10 commits into from
Aug 2, 2023

Conversation

queengooborg
Copy link
Collaborator

This PR sets the code language for various codeblocks that don't have one set, which will help us enforce setting a language on all codeblocks. This is the first part.

@queengooborg queengooborg requested review from a team as code owners July 24, 2023 07:04
@queengooborg queengooborg requested review from dipikabh, willdurand, sideshowbarker and teoli2003 and removed request for a team July 24, 2023 07:04
@github-actions github-actions bot added Content:WebExt WebExtensions docs Content:WebAPI Web API docs Content:Learn Learning area docs Content:Learn:Django Learning area Django docs Content:Learn:Express Learning area Express docs Content:Glossary Glossary entries Content:Learn:Client-side Content under “Client-side JavaScript frameworks” (Svelte, React, Angular, Vue) and related subtrees Content:Learn:JavaScript Learning area JavaScript docs Content:Firefox Content in the Mozilla/Firefox subtree Content:Meta Content in the meta docs Content:Learn:CSS Learning area CSS docs Content:Learn:HTML Learning area HTML docs Content:Learn:Forms Learning area Forms docs labels Jul 24, 2023
Josh-Cena
Josh-Cena previously requested changes Jul 24, 2023
View reviewed changes
Copy link
Member

@Josh-Cena Josh-Cena 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.

Hi @queengooborg, confusing as it sounds, "plain" is actually a marker for "it should have a language but the language it should be isn't supported so we use a placeholder". In Yari, "plain" is "planned for deprecation":

https://github.com/mdn/yari/blob/1374a7400b50610ea2a4dd1f6545cd613d360966/build/syntax-highlight.ts#L77

I'm on mobile, but I bet you can find past discussions on the content repo about this decision.

@queengooborg
Copy link
Collaborator Author

queengooborg commented Jul 24, 2023 •
edited
Loading

I'm -1 on deprecating "plain" as a language to apply to code blocks.

As I have been going through these files, I've found numerous instances where a language wasn't selected for syntax highlighting, even though it should be. There's probably a bunch marked as "plain" that shouldn't be as well, but by requiring a language to be specified, we can perform a simple search to find such instances in our documents and fix them. Additionally, enforcing some language to be set forces the author to think about what language the code block should be set to, so that we can catch issues early on. (Take note on how many code blocks I have specified a language other than "plain" for in this PR.)

Edit: our writing guidelines also say that "plain" should be used as a fallback: https://developer.mozilla.org/en-US/docs/MDN/Writing_guidelines/Howto/Markdown_in_MDN#example_code_blocks

@sideshowbarker sideshowbarker removed their request for review July 24, 2023 23:44
@queengooborg queengooborg mentioned this pull request Jul 24, 2023
Merged
@Josh-Cena Josh-Cena dismissed their stale review July 25, 2023 18:24

Consider myself convinced, then—would be helpful if you could later update that Yari comment to make the wording different.

@rubiesonthesky
Copy link
Contributor

I think this discussion is also been done multiple times, but is marking command line prompt and output as bash really the best option? In some instances you may get some syntax highlighting but in some none.

@queengooborg
Copy link
Collaborator Author

Good catch, @rubiesonthesky -- per our guidelines, we shouldn't be including the output of commands in Shell code blocks anyways. I'll submit a follow-up PR to fix this!

@github-actions github-actions bot added the merge conflicts 🚧 [PR only] label Jul 31, 2023
@github-actions
Copy link
Contributor

This pull request has merge conflicts that must be resolved before it can be merged.

@github-actions github-actions bot removed the merge conflicts 🚧 [PR only] label Aug 1, 2023
@queengooborg
Copy link
Collaborator Author

Still looking to get review on this PR so we can land it soon!

Josh-Cena
Josh-Cena reviewed Aug 2, 2023
View reviewed changes
Copy link
Member

@Josh-Cena Josh-Cena left a comment

Choose a reason for hiding this comment

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

Almost there!

files/en-us/web/api/fileentrysync/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/fileentrysync/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/mousescrollevent/index.md Outdated Show resolved Hide resolved
files/en-us/learn/javascript/building_blocks/looping_code/index.md Outdated Show resolved Hide resolved
Josh-Cena
Josh-Cena approved these changes Aug 2, 2023
View reviewed changes
Copy link
Member

@Josh-Cena Josh-Cena left a comment

Choose a reason for hiding this comment

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

LGTM. Let's get it in!

@Josh-Cena Josh-Cena merged commit 8d0cbea into main Aug 2, 2023
7 checks passed
@Josh-Cena Josh-Cena deleted the code-language/1 branch August 2, 2023 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bsmth bsmth bsmth left review comments

@Josh-Cena Josh-Cena Josh-Cena approved these changes

@dipikabh dipikabh Awaiting requested review from dipikabh dipikabh is a code owner automatically assigned from mdn/core-yari-content

@willdurand willdurand Awaiting requested review from willdurand willdurand is a code owner automatically assigned from mdn/yari-content-mozilla-add-ons

@teoli2003 teoli2003 Awaiting requested review from teoli2003 teoli2003 is a code owner automatically assigned from mdn/yari-content-http

@hamishwillee hamishwillee Awaiting requested review from hamishwillee

Assignees
No one assigned
Labels
Content:Firefox Content in the Mozilla/Firefox subtree Content:Glossary Glossary entries Content:Learn:Client-side Content under “Client-side JavaScript frameworks” (Svelte, React, Angular, Vue) and related subtrees Content:Learn:Cross-Browser-Testing Learning area Cross-Browser-Testing docs Content:Learn:CSS Learning area CSS docs Content:Learn:Django Learning area Django docs Content:Learn:Express Learning area Express docs Content:Learn:Forms Learning area Forms docs Content:Learn:HTML Learning area HTML docs Content:Learn:JavaScript Learning area JavaScript docs Content:Learn Learning area docs Content:Meta Content in the meta docs Content:WebAPI Web API docs Content:WebExt WebExtensions docs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@queengooborg @rubiesonthesky @bsmth @Josh-Cena