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

Reword m flag description #37711

Merged
merged 3 commits into from
Jan 24, 2025
Merged

Reword m flag description #37711

merged 3 commits into from
Jan 24, 2025

Conversation

DanKaplanSES
Copy link
Contributor

@DanKaplanSES DanKaplanSES commented Jan 19, 2025

Description

Reword m flag description

Motivation

I didn't understand what m meant from this page's description, so I reworded it. Hopefully this is more clear.

Additional details

Related issues and pull requests

I didn't understand what `m` meant from this page's description, so I reworded it. Hopefully this is more clear.
@DanKaplanSES DanKaplanSES requested a review from a team as a code owner January 19, 2025 03:58
@DanKaplanSES DanKaplanSES requested review from Josh-Cena and removed request for a team January 19, 2025 03:58
@github-actions github-actions bot added Content:JS JavaScript docs size/xs [PR only] 0-5 LoC changed labels Jan 19, 2025
Copy link
Contributor

github-actions bot commented Jan 19, 2025

Preview URLs

(comment last updated: 2025-01-24 18:14:58)

@Josh-Cena
Copy link
Member

How do you like this wording? Mostly want to align with dotAll

@DanKaplanSES
Copy link
Contributor Author

DanKaplanSES commented Jan 19, 2025

How do you like this wording? Mostly want to align with dotAll

I think I should share context that led me to submit this PR: I've read this table many times before, but I never really understood what the m flag did. I think that's because whenever I looked up ^ or $, it's always explained in terms of lines.

So whenever I read Allows ^ and $ to match next to newline characters. (the original wording), I always wondered, "Isn't that the way it always works?"

I now know that ^ and $ match the beginning/end of the entire string by default; it's the m flag that makes these work on every line.

The new wording is, Allows ^ and $ to match the start and end of each line. I think that's a big improvement over the original, but I still would be left wondering, "Isn't that the way it always works?"

What would you think about changing it to Allows ^ and $ to match the start and end of each line instead of the entire string.? From my perspective, it seems like the best of both worlds. That said, even without my suggestion, your modification is a big improvement, so I'll defer to you.

Thanks as always!

@Josh-Cena
Copy link
Member

I'm okay with that. I originally went with your wording, but it was slightly too long and caused the entire table to grow, but if you agree with it let's leave that concern behind.

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.

I've applied your suggestion—with some minor tweaks. Thanks!

@Josh-Cena Josh-Cena merged commit ed8ab20 into mdn:main Jan 24, 2025
8 checks passed
@DanKaplanSES
Copy link
Contributor Author

@Josh-Cena Looks good! I think those of could be removed from the description without losing any meaning. If you think that's worthwhile, let me know and I'll submit a new PR.

@Josh-Cena
Copy link
Member

I decided to add that specifically to disambiguate. Otherwise I personally could read it as "match the start and end of each line, instead of matching the entire string." I just don't want someone to send an issue saying it doesn't do what it says.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:JS JavaScript docs size/xs [PR only] 0-5 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants