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

Switch to using Markdown code fence notation instead of shortcodes and update style guide accordingly. #45

Merged

Conversation

coffee2code
Copy link
Contributor

  • Switches to using Markdown code fence notation instead of shortcodes
  • Removes HTML entity substitutions as they are no longer necessary.

Style guide:

  • Removes 'Shortcodes' section as shortcodes are no longer recommended. Markdown code fences, as described in prior section, is the preferred method.
  • Escapes the code fence examples so the Markdown code fence notation themselves are shown. The examples intend to demonstrate the use of code fences (often with language hinting) but when unescaped this results in the code being formatted instead of shown.
  • Adds an 'HTML' example to the 'Fenced Code Blocks' section as that appears to have been an oversight.

Fixes #44.

Pages look better when viewed as Markdown and, more importantly, are imported into handbooks with better formatting retention.

Also removes HTML entity substitutions as they are no longer necessary.
…e fences.

* Removes 'Shortcodes' section as shortcodes are no longer recommended. Markdown code fences, as described in prior section, is the preferred method.
* Escapes the code fence examples so the Markdown code fence notation themselves are shown. The examples intend to demonstrate the use of code fences (often with language hinting) but when unescaped this results in the code being formatted instead of shown.
* Adds an 'HTML' example to the 'Fenced Code Blocks' section as that appears to have been an oversight.

Fixes WordPress#44.
Copy link
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

@coffee2code Looks good. I left some inline remarks for you to consider.

It would be amazing if this finally solved the code sample display issue 🤞

Honest question: throughout the rest of the text, HTML is still used. Should/can the rest of the text now be converted to markdown too ? (as per the styleguide)

styleguide.md Outdated

### Fenced Code Blocks

#### Javascript
````
```js
Copy link
Member

Choose a reason for hiding this comment

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

The actual pages now use triple-backtick + javascript, while here in the style guide it shows to use triple backtick + js. Which is correct ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! This was the pre-existing recommendation that I neglected to update.

elements
.addClass( 'foo' )
.children()
.html( 'hello' )
.end()
.appendTo( 'body' );
[/javascript]
```
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this fencing closer be at the start of the line ?

&042;/
```css
/**
* #.# Section title
Copy link
Member

@jrfnl jrfnl Sep 25, 2020

Choose a reason for hiding this comment

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

These lines all need a space at the start (before the *) I think, to align the stars.

Copy link
Member

Choose a reason for hiding this comment

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

@@ -530,19 +530,19 @@ var arr = _.chain( obj )
// Exit the chain
.value();

// arr === [ 'first comes thing 1', 'second comes thing 2', 'third comes lox' ][/javascript]
// arr === [ 'first comes thing 1', 'second comes thing 2', 'third comes lox' ]```
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this fencing closer be on a line by itself ?

[tab]$foo5 = 'somevalue4';
[/php]
```php
$foo = 'somevalue';
Copy link
Member

Choose a reason for hiding this comment

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

The [tab] markings were there on purpose to show tabs should be used not spaces.

$args = array(
[tab]'post_type' => 'page',
Copy link
Member

Choose a reason for hiding this comment

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

Dito.

[tab]case 'bar':
[tab][tab]some_function();
[tab][tab]break;
case 'foo':
Copy link
Member

Choose a reason for hiding this comment

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

Dito.

Copy link
Member

@ntwb ntwb left a comment

Choose a reason for hiding this comment

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

Thanks @coffee2code, fingers crossed when this syncs 😏

@jrfnl Comments covered it all I can see, update and merge away 💯

* Correct styleguide to recommend 'javascript' as language hint for code fences (as used in all examples)
* Restore `[tab]` as they are intended to be rendered as-is
* Add space before multiline comment to align asterisks
* Fix errant positioning of closing code fences
Copy link
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

LGTM!

@ntwb
Copy link
Member

ntwb commented Oct 11, 2020

Merging this in now, will check how it all looks after sync shortly

@ntwb ntwb merged commit eb0aa61 into WordPress:master Oct 11, 2020
@ntwb
Copy link
Member

ntwb commented Oct 12, 2020

Checking various pages, the only issue I'm seeing as single quotes using ' rather than '
These still have some issues:

* Description including a code sample:
*
*    $status = array(
*        'draft'   => __( 'Draft' ),
*        'pending' => __( 'Pending Review' ),
*        'private' => __( 'Private' ),
*        'publish' => __( 'Published' )
*    );
*
* The description continues on ...

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.

Use Markdown code fence notation instead of shortcodes to denote code blocks
3 participants