Skip to content

Raw handling: Skip shortcode if not on its own line#19059

Merged
mcsf merged 1 commit intomasterfrom
fix/shortcode-inline-detection
Dec 12, 2019
Merged

Raw handling: Skip shortcode if not on its own line#19059
mcsf merged 1 commit intomasterfrom
fix/shortcode-inline-detection

Conversation

@mcsf
Copy link
Copy Markdown
Contributor

@mcsf mcsf commented Dec 11, 2019

Description

Improves the existing logic for detection of "inline shortcodes" by ensuring that a shortcodes stands not only at the start of a line/paragraph, but also at the end of one.

Example

The following classic input:

[foo]A[/foo] [foo]B[/foo]

yields:

Before After
Shortcode block for [foo]A[/foo] followed by Paragraph block with content [foo]B[/foo]. Paragraph block with content equal to input: [foo]A[/foo] [foo]B[/foo].

How has this been tested?

Addition of integration tests.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

Improves the existing logic for detection of "inline shortcodes" by
ensuring that a shortcodes stands not only at the start of a
line/paragraph, but also at the end of one.
@mcsf mcsf added [Feature] Paste [Feature] Shortcodes Related to shortcode functionality labels Dec 11, 2019
@mcsf mcsf requested a review from ellatrix December 11, 2019 11:10
@mcsf mcsf requested a review from youknowriad as a code owner December 11, 2019 11:10
! /(\n|<p>)\s*$/.test( beforeHTML )
! (
/(\n|<p>)\s*$/.test( beforeHTML ) &&
/^\s*(\n|<\/p>)/.test( afterHTML )
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This could be non capturing?

@ellatrix
Copy link
Copy Markdown
Member

Any issues that this fixes?

@mcsf
Copy link
Copy Markdown
Contributor Author

mcsf commented Dec 11, 2019

Any issues that this fixes?

Nothing tracked to my knowledge — just something I stumbled upon.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Minor: Maybe here too?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Actually, on second thought I'm going to roll back on the non-capturing group changes. I'm thinking that the fact that we're testing with RegExp::test instead of ::exec or String::match should be enough — and if not, that's something for the VMs to work on!

Quoting from RegExp::test:

Use test() whenever you want to know whether a pattern is found in a string. test() returns a boolean, unlike the String.prototype.search() method, which returns the index (or -1 if not found). To get more information (but with slower execution), use the exec() method (similar to the String.prototype.match() method). As with exec() (or in combination with it), test() called multiple times on the same global regular expression instance will advance past the previous match.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sounds good! Thanks for looking into it! :)

@mcsf mcsf force-pushed the fix/shortcode-inline-detection branch from 96ba092 to c4b361e Compare December 12, 2019 11:48
@mcsf mcsf merged commit da6d2c2 into master Dec 12, 2019
@mcsf mcsf deleted the fix/shortcode-inline-detection branch December 12, 2019 11:51
@youknowriad youknowriad added this to the Gutenberg 7.2 milestone Jan 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Feature] Paste [Feature] Shortcodes Related to shortcode functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants