Skip to content

Conversation

@dark0dave
Copy link
Contributor

@dark0dave dark0dave commented Dec 18, 2025

@FredrikLindgren this removes as much as possible any old functions which are called by weidu. Should reduce the noise in the build stage too.

@dark0dave dark0dave changed the title feat(strings): Remove depricated string functions or at least make a start feat(strings): Remove depricated string functions Dec 18, 2025
@FredrikLindgren
Copy link
Member

FredrikLindgren commented Dec 19, 2025

This is unfortunately one of those types of changes I'm going to be difficult about. Since, say, uppercase is not semantically equivalent to uppercase_ascii it is not necessarily backward-compatible to replace the former with the latter. For all I know, there are mods out there that rely on the case transformation being done in Latin1 and would break otherwise.

There definitely are cases where it is safe to switch: the ones where the input is known, say, up-casing MODDER arguments, and in those cases I'm not arguing. But if the case-transform is on unknown input it's a different matter, for which I feel there is not necessarily a good answer. At the very least, I'd do these in a different commit, so the mod-facing changes could be reverted in case of reports of mod breakage.

I'm not suggesting you should do this, but in the alternate universe in which I actually accomplish anything, I would have combined this review with a critical eye towards whether or not the case-transform was necessary or could be done some other way, especially wrt. linux compatibility (for instance, making a biff up-cases the contents your key-file which is a no-no). Edit: not so much actually doing these types of changes, but charting where work is necessary.

@dark0dave
Copy link
Contributor Author

dark0dave commented Dec 20, 2025

@FredrikLindgren ah, I didn't consider that! You are right! My apologies. Would you be open to making a function instead? Just so we can get closer to a new version of ocaml?

@dark0dave
Copy link
Contributor Author

dark0dave commented Dec 20, 2025

Some thing like:

let latin1_to_upper (s: string) : string =
  String.map (fun c ->
    if c >= '\x61' && c <= '\x7A' then
      char_of_int (int_of_char c - 32)
    else
      c
  ) s

@dark0dave dark0dave marked this pull request as draft December 21, 2025 14:07
@dark0dave dark0dave changed the title feat(strings): Remove depricated string functions Draft: feat(strings): Remove depricated string functions Dec 21, 2025
@FredrikLindgren
Copy link
Member

So it seems like mine was perhaps a bit of a void concern. I don't know the inner workings of, say, String.uppercase and String.uppercase_ascii, but it seems like they behave identically on Latin-1 input, that is to say, they just uppercase a-z and leavé any code pöints in the non-ASCII range alone. There ought to be some difference between the two, but I can't find any documentation that retains String.uppercase, and empirical evidence is thus far to the contrary. Still, I would feel better if there was one commit that did this change for known inputs and another that handle the cases where the input is unknown. I can do it, as it is much to ask, but then it's on a more of a glacial time scale.

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.

2 participants