Skip to content

Remove the places where warning 3 was raised in str.#581

Closed
bschommer wants to merge 2 commits intoocaml:trunkfrom
bschommer:str-warning-clean
Closed

Remove the places where warning 3 was raised in str.#581
bschommer wants to merge 2 commits intoocaml:trunkfrom
bschommer:str-warning-clean

Conversation

@bschommer
Copy link
Copy Markdown
Contributor

There were a few places where the lowercase/uppercase functions where used.

@alainfrisch
Copy link
Copy Markdown
Contributor

This changes the semantics of Str. There could be people relying on its current Latin-1 oriented semantics...

@bschommer
Copy link
Copy Markdown
Contributor Author

That is right. Maybe one wants to duplicate the function with the ascii and latin-1 version?

@damiendoligez
Copy link
Copy Markdown
Member

cc @xavierleroy

@DemiMarie
Copy link
Copy Markdown
Contributor

OCaml really needs UTF-8 versions (which are what people almost always need). However, Unicode breaks so many assumptions that this would not be backwards-compatible – in general, the only way to correctly handle Unicode (unless you are doing something simple like escaping ASCII metacharacters) is with a library designed for the purpose.

@damiendoligez damiendoligez added this to the 4.04 milestone Jun 7, 2016
Similar to the String, Char and Bytes module the latin1 version of the
function is now marked as depricated and version for the ascii
character set are added.
@bschommer
Copy link
Copy Markdown
Contributor Author

I updated this PR to include ascii version of the case insensitive regexp matching. There are also some test for the new versions.

@xavierleroy
Copy link
Copy Markdown
Contributor

I'm still unsure what to do about this one. My inner voice says that we should just silently remove Latin-1 support from the existing functions, not create new "_ascii" functions. But that's not the direction that was taken in the standard library. Should I tow the party line? or join the dissidents? Still undecided.

@bschommer
Copy link
Copy Markdown
Contributor Author

The problem with just changing the implementation is that it will probably break existing code. However since it was never documented what uppercase/lowercase functions were used it would not be that problematic.

Alternatively one could just expose the internal function and allow users to pass a fold_case and a fold_stringfunction to the functions.

@dra27
Copy link
Copy Markdown
Member

dra27 commented Dec 4, 2016

Sorry if it's been previously discussed, but does a change like this make a case for cutting Str out of the standard distribution? That way conforming to the behaviour becomes tied to the version of the library, rather than OCaml?

@xavierleroy
Copy link
Copy Markdown
Contributor

@dra27: yes, sending dissidents abroad is one way to preserve the party line...

@dra27
Copy link
Copy Markdown
Member

dra27 commented Dec 4, 2016

😄 I was thinking it could be made compliant and sent abroad!

@shindere
Copy link
Copy Markdown
Contributor

shindere commented Dec 5, 2016 via email

@xavierleroy
Copy link
Copy Markdown
Contributor

If your problem is just to see no warning, just put the correct [@ocaml.warning] attributes to silence the warning at the points where Char.lowercase and Char.uppercase are used. We know they are used, purposefully, and don't need constant reminder.

@bschommer
Copy link
Copy Markdown
Contributor Author

Removing the warnings with the correct [@ocaml.warning] would be okay. But additionally one maybe wants to document that these functions use the latin-1 versions to avoid confusion.

@shindere
Copy link
Copy Markdown
Contributor

shindere commented Dec 5, 2016 via email

@bschommer bschommer closed this Dec 5, 2016
@bschommer bschommer deleted the str-warning-clean branch December 5, 2016 15:11
EduardoRFS pushed a commit to esy-ocaml/ocaml that referenced this pull request Jul 23, 2021
Move our usage of inline to Caml_inline (to align with upstream ocaml)
stedolan pushed a commit to stedolan/ocaml that referenced this pull request May 24, 2022
EmileTrotignon pushed a commit to EmileTrotignon/ocaml that referenced this pull request Jan 12, 2024
…heir sub-pages (ocaml#581)

* factoring out shared elements between 'Learn' page and 'Exercises' page: `Learn_layout` now exports functions that render the links and link groups, as well as the icon link group of the sidebar
* refactoring Learn_layout.eml into a layout component
* "Documentation" in sidebar is now "Learn OCaml"
* minor style improvements

Co-authored-by: Sabine Schmaltz <sabine@tarides.com>
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.

7 participants