Fix description of regex flags by Josh-Cena · Pull Request #19540 · mdn/content

Josh-Cena · 2022-08-14T08:19:15Z

Summary

Motivation

Fix #1454 (finally!)
Fix #19524 — flags are changed to accessor properties in ES6.

Supporting details

Related issues

Metadata

Adds a new document
Rewrites (or significantly expands) a document
Fixes a typo, bug, or other error

github-actions · 2022-08-14T08:38:49Z

Preview URLs

Flaws

None! 🎉

External URLs

URL: /en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/global
Title: RegExp.prototype.global
on GitHub

No new external URLs

URL: /en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/unicode
Title: RegExp.prototype.unicode
on GitHub

No new external URLs

URL: /en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/source
Title: RegExp.prototype.source
on GitHub

No new external URLs

URL: /en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/flags
Title: RegExp.prototype.flags
on GitHub

No new external URLs

URL: /en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/sticky
Title: RegExp.prototype.sticky
on GitHub

No new external URLs

URL: /en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/hasIndices
Title: RegExp.prototype.hasIndices
on GitHub

No new external URLs

URL: /en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/dotAll
Title: RegExp.prototype.dotAll
on GitHub

No new external URLs

URL: /en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/ignoreCase
Title: RegExp.prototype.ignoreCase
on GitHub

https://unicode-org.github.io/icu/userguide/transforms/casemappings.html (1 time) (Note! This may be a new URL 👀)
https://unicode.org/Public/UCD/latest/ucd/CaseFolding.txt (1 time) (Note! This may be a new URL 👀)

URL: /en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/multiline
Title: RegExp.prototype.multiline
on GitHub

No new external URLs

URL: /en-US/docs/Web/JavaScript/Guide/Regular_Expressions
Title: Regular expressions
on GitHub

No new external URLs

(this comment was updated 2022-08-30 05:40:13.181025)

ghost · 2022-08-14T08:59:29Z

files/en-us/web/javascript/reference/global_objects/regexp/unicode/index.md

-You cannot change this property directly. It is read-only.
+- Any [Unicode codepoint escapes](/en-US/docs/Web/JavaScript/Guide/Regular_Expressions/Unicode_Property_Escapes) (`\u{xxxx}`, `\p{UnicodePropertyValue}`) will be interpreted as such instead of literal characters.
+- Surrogate pairs will be interpreted as whole characters instead of two separate characters. For example, `/[😄]/u` would only match `"😄"` but not `"\ud83d"`.
+- When [`lastIndex`](/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/lastIndex) is automatically advanced (such as when calling [`exec()`](/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/exec)), unicode regexps advance by Unicode codepoints instead of UTF-16 code units.


Hmm, I'd think that a warning about the mismatch between JavaScript string indexing with brackets vs. the index returned here, might be warranted?

lastIndex still returns the UTF-16 index. It just may advance by 2 in case of a surrogate pair.

ghost · 2022-08-14T09:03:27Z

files/en-us/web/javascript/reference/global_objects/regexp/sticky/index.md

@@ -1,5 +1,5 @@
 ---
-title: RegExp.prototype.sticky
+title: get RegExp.prototype.sticky


Ooh, I have not seen get used in any MDN titles yet... isn't such an introduction pending the discussion about prototype accessors vs own properties?

No, get is already used in @@species. I think it makes sense to update these.

Was that broadly agreed in @@species or did it just just sneak in?

I have no care factor about what syntax we use, but it shouldn't appear by magic - it should be discussed (and not just in one PR). If agreed it should be updated in the contributor docs so that we're all on the same page. IN addition, we should identify all the cases that need updating and do them as a project.

If there was extensive discussion and this is all documented and well understood. Apologies. Don't know how I missed it.

I made a similar comment above - just linking so no need to respond in both places #19540 (comment)

@@species seems to just have been like that from the start. Plus we just don't have an agreed-upon notation for any property. (cf mdn/mdn-community#153) I guess @@species was written like that because these pages are so niche that no one cares about them other than spec gurus :-)

I really don't have strong opinions about whether we prefix getter-only properties with get, but this seems like a really low-bar change compared to marking up instance properties. If others feel like the change is too bold I can revert it for now.

ghost · 2022-08-14T09:04:38Z

files/en-us/web/javascript/reference/global_objects/regexp/sticky/index.md

 ## Description

-The value of `sticky` is a {{jsxref("Boolean")}} and true if the `y` flag was used; otherwise, false. The `y` flag indicates that it matches only from the index indicated by the {{jsxref("RegExp.lastIndex", "lastIndex")}} property of this regular expression in the target string (and does not attempt to match from any later indexes). A regular expression defined as both `sticky` and `global` ignores the `global` flag.
+`RegExp.prototype.sticky` is a getter-only property that returns `true` if the `y` flag was used; otherwise, `false`. The `y` flag indicates that it matches only from the index indicated by the {{jsxref("RegExp.lastIndex", "lastIndex")}} property of this regular expression in the target string (and does not attempt to match from any later indexes).


I suspect that the term "getter-only" may not be friendly towards beginners. Any thoughts on this?

The fact that no-one complains up till now probably means these pages are non-starter from the beginning, so I guess it's fine. Most would read the guide anyway.

How is "getter-only" different from read-only? i.e. it is far more common to see something like:

The RegExp.prototype.sticky is a read-only property returns true ...

And in the parent doc you'd have the {{readonly_inline}} macro

Not much. "Read-only" is for data properties, "getter-only" is for accessor properties; but when you use them they are pretty much interchangeable.

I'm just not sure it is a useful distinction. If it isn't, then better to make things consistent. If it is, then great - but we need to agree how and document that approach.

Edited. I see this has been added to the discussion. Let's finish this there.

Let's discuss in mdn/mdn-community#195. If others also feel strongly about this change I can revert it (at best aligning with what we do with other getter-only properties like %TypedArray%.p.length) so we can get the important pieces in.

Absolutely.

hamishwillee · 2022-08-15T00:03:13Z

files/en-us/web/javascript/reference/global_objects/regexp/dotall/index.md

@@ -1,8 +1,7 @@
 ---
-title: RegExp.prototype.dotAll
+title: get RegExp.prototype.dotAll


I have never seen get used in a title in MDN.

Generally JavaScript has been treated as a little bit special in that it remains closer to spec language that the web api docs. In my strong opinion this might be fine, but it should be raised as a discussion and agreed, not added by magic.

@wbamberg might have an opinion

Further, if we're going to change the rules, then they should be changed across the docset and reflected in the template guidance for javascript thingies.

It is possible this has been done of course, and I just missed it.

Further, if we're going to change the rules, then they should be changed across the docset and reflected in the template guidance for javascript thingies.

It is possible this has been done of course, and I just missed it.

See #19540 (comment)

Yeah, I agree with Hamish that we should do this in an agreed and organized way. Without that I'm -1 on get here.

Josh-Cena · 2022-08-26T03:37:43Z

I've hopefully made the content a little clearer and reverted the title changes. I intend to re-propose this sometime in the future when I have more energy.

Josh-Cena · 2022-08-27T16:04:25Z

Minor ping for @wbamberg in case you have any feedback on this ^^ The wording change here is pretty important and it's a shame to be delayed so long (the issue has remained for a year and a half)

wbamberg

Mostly copy-edits, though I had a bigger comment on the sticky page. But it's a use-it-or-not-as-you-wish type of thing.

files/en-us/web/javascript/guide/regular_expressions/index.md

files/en-us/web/javascript/reference/global_objects/regexp/hasindices/index.md

files/en-us/web/javascript/reference/global_objects/regexp/ignorecase/index.md

files/en-us/web/javascript/reference/global_objects/regexp/unicode/index.md

files/en-us/web/javascript/reference/global_objects/regexp/source/index.md

files/en-us/web/javascript/reference/global_objects/regexp/sticky/index.md

Co-authored-by: wbamberg <will@bootbonnet.ca>

wbamberg

👍 thank you @Josh-Cena !

* Fix description of regex flags * fix * Fix source * Better wording * Add link * Apply suggestions from code review Co-authored-by: wbamberg <will@bootbonnet.ca> * Add an exhaustive list Co-authored-by: wbamberg <will@bootbonnet.ca>

Josh-Cena requested a review from a team as a code owner August 14, 2022 08:19

Josh-Cena requested review from wbamberg and removed request for a team August 14, 2022 08:19

github-actions bot added the Content:JS JavaScript docs label Aug 14, 2022

Josh-Cena force-pushed the fix-regex-flag branch 3 times, most recently from a5f64ef to ab4433a Compare August 14, 2022 08:27

Fix description of regex flags

86349af

Josh-Cena force-pushed the fix-regex-flag branch from ab4433a to 86349af Compare August 14, 2022 08:28

fix

bd3b640

Fix source

808756e

ghost reviewed Aug 14, 2022

View reviewed changes

hamishwillee reviewed Aug 15, 2022

View reviewed changes

Josh-Cena force-pushed the fix-regex-flag branch from d41e04a to d556ddd Compare August 26, 2022 03:41

Better wording

383f065

Josh-Cena force-pushed the fix-regex-flag branch from d556ddd to 383f065 Compare August 26, 2022 03:43

Add link

d8c234b

Josh-Cena force-pushed the fix-regex-flag branch from f83f23a to d8c234b Compare August 28, 2022 03:59

wbamberg requested changes Aug 30, 2022

View reviewed changes

Apply suggestions from code review

c739b43

Co-authored-by: wbamberg <will@bootbonnet.ca>

Josh-Cena force-pushed the fix-regex-flag branch from 20ab807 to 7090a2c Compare August 30, 2022 05:24

Josh-Cena requested a review from wbamberg August 30, 2022 05:25

Josh-Cena force-pushed the fix-regex-flag branch from 7090a2c to 7b0c81c Compare August 30, 2022 05:36

Add an exhaustive list

1d04238

Josh-Cena force-pushed the fix-regex-flag branch from 7b0c81c to 1d04238 Compare August 30, 2022 05:37

OnkarRuikar mentioned this pull request Aug 30, 2022

Lint only modified files #19884

Closed

wbamberg approved these changes Aug 30, 2022

View reviewed changes

wbamberg merged commit 7a1edd4 into mdn:main Aug 30, 2022

Josh-Cena deleted the fix-regex-flag branch August 30, 2022 21:21

Josh-Cena mentioned this pull request Nov 23, 2022

[page types] Define page types for JavaScript #16156

Closed

github-actions bot locked as resolved and limited conversation to collaborators Sep 1, 2023

Conversation

Josh-Cena commented Aug 14, 2022

Summary

Motivation

Supporting details

Related issues

Metadata

Uh oh!

github-actions bot commented Aug 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Preview URLs

Flaws

External URLs

Uh oh!

ghost Aug 14, 2022 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Josh-Cena Aug 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hamishwillee Aug 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Josh-Cena commented Aug 26, 2022

Uh oh!

Josh-Cena commented Aug 27, 2022

Uh oh!

wbamberg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wbamberg left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Aug 14, 2022 •

edited

Loading

ghost Aug 14, 2022 •

edited by ghost

Loading

Josh-Cena Aug 15, 2022 •

edited

Loading

hamishwillee Aug 15, 2022 •

edited

Loading