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

Update perlfaq4.pod to use fc for safe lowercase comparisons #102

Merged
merged 3 commits into from
Jul 1, 2023
Merged

Update perlfaq4.pod to use fc for safe lowercase comparisons #102

merged 3 commits into from
Jul 1, 2023

Conversation

briandfoy
Copy link

No description provided.

Copy link
Member

@bigpresh bigpresh left a comment

Choose a reason for hiding this comment

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

Would it be worth a brief explanation on why you'd use fc (case folding) rather than lc?

e.g. something like:

"Note - you'll often see code which uses lc instead, e.g. lc $a cmp lc $b - whilst that will work, it can be tripped up by certain Unicode characters, and fc(), added in 5.16, should be preferred."

and probably also a note that fc() is only available if you've explicitly enabled it with feature, or you've said use v5.16; or higher? Otherwise, someone following this who only said use strict or is updating legacy code with no version dep will just get tripped up. e.g. the note from fc's entry in perlfunc:

fc is available only if the "fc" feature is enabled or if it is prefixed with CORE::. The "fc" feature is enabled automatically with a use v5.16 (or higher) declaration in the current scope.

Note that fc() is a v5.16 features.
@davorg
Copy link
Member

davorg commented Jun 23, 2023

New version looks good to me. I say ship it.

davorg
davorg previously approved these changes Jun 23, 2023
bigpresh
bigpresh previously approved these changes Jun 23, 2023
@davorg
Copy link
Member

davorg commented Jun 23, 2023

Are we expecting the Travis checks to complete? It looks like I don't have permission to skip them and merge.

lib/perlfaq4.pod Outdated
might be affected by your locale settings). The output list has the keys
in ASCIIbetical order. Once we have the keys, we can go through them to
create a report which lists the keys in ASCIIbetical order.
keys to the sort function which then compares them strings. The output list
Copy link

Choose a reason for hiding this comment

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

"compares them strings" -> "compares them as strings"

Copy link

Choose a reason for hiding this comment

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

Note that without "use locale", the comparison order may be better described as ASCIIbetical, and not how one would expect locale-based string comparisons. Not that this answer necessarily needs to detail that.

@briandfoy briandfoy dismissed stale reviews from bigpresh and davorg via f6d556a June 23, 2023 17:23
@karenetheridge karenetheridge merged commit 79872a9 into perl-doc-cats:master Jul 1, 2023
karenetheridge added a commit that referenced this pull request Jul 1, 2023
  * Fix Unicode code point range in glossary (Felipe Gasper, #98)
  * faq4: recommend fc() for string sorting, rather than lc() (brian d foy,
    #102)
@karenetheridge
Copy link
Member

thanks, just released!

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.

None yet

5 participants