Skip to content

perlop: Fixups #23286

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

Open
wants to merge 16 commits into
base: blead
Choose a base branch
from
Open

perlop: Fixups #23286

wants to merge 16 commits into from

Conversation

khwilliamson
Copy link
Contributor

Fixes #18565

  • This set of changes does not require a perldelta entry.

@khwilliamson
Copy link
Contributor Author

In re-reading the changes, I realized the old text wrongly used "binary" instead of "logical". Also that I had wrongly said xor is the equivalent of ^. It instead is the equivalent of ^^ (which I hadn't known existed).

Grinnz
Grinnz previously requested changes May 14, 2025
Copy link
Contributor

@Grinnz Grinnz left a comment

Choose a reason for hiding this comment

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

I think or and xor should be kept as the same section, and the "binary" term should not be changed, otherwise LGTM

@Grinnz
Copy link
Contributor

Grinnz commented May 14, 2025

Also that I had wrongly said xor is the equivalent of ^. It instead is the equivalent of ^^ (which I hadn't known existed).

It had not existed, until the 5.40 release. https://perldoc.perl.org/perl5400delta#New-%5E%5E-logical-xor-operator

@khwilliamson
Copy link
Contributor Author

It turns out that there a bunch of infelicitous things in this pod; so I'm working on a more extensive fix

@khwilliamson khwilliamson changed the title perlop: Add detail about xor perlop: Fixups May 17, 2025
@khwilliamson khwilliamson dismissed Grinnz’s stale review May 17, 2025 17:59

fixed as requested

@khwilliamson
Copy link
Contributor Author

I found a bunch of problems/improvements to be made to this pod

I wonder if the smartmatch text needs updating.

X<operator, logical, or> X<or>
X<operator, logical, xor> X<operator, logical, exclusive or> X<xor>

There is no low precedence operator for defined-OR.
Copy link
Contributor

Choose a reason for hiding this comment

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

This feels slightly better at the end of this section, as an "addendum," in my opinion at least

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How would that read? I think the current situation is that it gets lost

Copy link
Contributor

Choose a reason for hiding this comment

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

My feeling is that people would more likely go to this section of the documentation to find out about the or and xor operators, more than to search for a low precedence defined-or, but I have no objections to putting that sentence first either.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Grinnz, I agree that that is why they would come here; but I moved the sentence because I thought that point would be lost where it was, and it's not really a problem to have it moved.

Now you say you don't object to moving the sentence. What changes need to be made, then?

Copy link
Contributor

Choose a reason for hiding this comment

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

Totally up to you, just giving my opinion on it

@Grinnz
Copy link
Contributor

Grinnz commented May 17, 2025

I wonder if the smartmatch text needs updating.

It should be updated since it is no longer considered experimental, and to describe the current state of the operator, which is that it is discouraged and unlikely to receive any further development. (may be suitable for a separate PR)

Enhancements have been made in the last few releases to using these
operators on UTF-8 locales; but this advice was not updated, so until
this commit, said don't do that.
This document is supposed to be in decreasing operator precedence order.
isa() is higher than the relational operators, so move it to there.
It is more customary to separate an 'if' from its paren.
Otherwise this dangling sentence gets lost after the main part of the
section.
These paragraphs were separated from the main area where all but one of
their subjects, 'cmp',  were discussed.  Move them, and provide a link
to the outlier, and back to the main discussion.
This has the same precedence as other operators, so should not be in a
separate section with the same heading level.

Solve this by making it at a sublevel.  This fits nicely with another
sublevel section on smartmatch that follows immediately.  There are now
to subsections.
This gets things more rationally ordered
This detail for two sections was combined in a whole other section.
This splits the detail into text appropriate for each section to which
it applies, and moves it there.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[doc] perlop: splitting of the section about 'or' and 'xor'
4 participants