Improve warning 14: illegal backslash. Fix #10929

[lucas-deangelis]

There's already some discussion in the linked issue, #10929. All tests pass locally, check-typo doesn't report any issue.

[nojb]

I find the hint about quoted strings confusing, as the example has no relation with code that triggers the warning. Personally I would remove it. Otherwise the PR looks fine to me.

[nojb]

Suggested change

	(\n, \r...). Did you check the list of OCaml escape sequences?
	(\n, \r, ...). Did you check the list of OCaml escape sequences?

[Octachron]

If I am not mistaken, @dra27 's idea for quoted strings was : if there is no valid escape sequence in the raw string, propose to use quoted strings with the raw string:

let test = "av\*"

would trigger

Hint: Did you mean {|av\*|}?

but not for example like

let ambiguous = "a\n\*"

[lucas-deangelis]

You're right, I misunderstood @dra27's idea. The mechanism for adding logic that depends on the string that raised the warning is less clear to me, so it would take me more time to make a full PR. In that case, would it be alright to split that into two PRs? The first would be this one, with the typo corrected and the mention of quoted strings removed. The second one would have the logic for suggesting quoted strings when no valid escape sequence is found in the raw string.

[nojb]

Personally I find the hint for quoted strings a bit too clever and would be fine without it, but I am not strongly opposed either.

[Octachron]

It is perfectly fine to split the potentially too clever quoted strings hint into another (potential) PR.

[dra27]

@nojb's change needs to be here (then in the reference files as well)

Suggested change

	(\\n, \\r...). Did you check the list of OCaml escape sequences?\n\
	(\\n, \\r, ...). Did you check the list of OCaml escape sequences?\n\

[dra27]

@Octachron is (of course!) correct about what I was trying to get at. The rationale for suggesting it is that it's useful when accidentally generating things like "C:\Windows\system32", but I completely agree that that can be for another PR, if at all (it was also clear from the Discourse thread references that using quoted strings for that is not as widely-known as might be nice).

[gasche]

When I play videogames, loading screens include random hints. What's the OCaml compiler equivalent of that? I guess we could ask editor modes to show a "Did you know: ..." banner once every day.

[nojb]

LGTM (modulo the missing space)

[nojb]

Suggested change

	(\n, \r,...). Did you check the list of OCaml escape sequences?
	(\n, \r, ...). Did you check the list of OCaml escape sequences?

[lucas-deangelis]

Sorry for the activity, I had some struggles with git. I like the idea of suggestions and hints too. In other languages, this tends to be in linters, but I'm not aware of any linter for OCaml.

[Octachron]

An important notice @lucas-deangelis: we are still sorting out the repository state after the multicore merge. The merge of non-OCaml 5.00 PRs is still suspended for a time, but your PR shall be merged once this freeze period is gone.

[lucas-deangelis]

Thank you for the notice. I've fixed the typo and completed the reviewers list.

[Octachron]

@lucas-deangelis , we are once again merging non-multicore PRs for OCaml. If you have the time to rebase your PR (to fix the Changes entry), it can now be merged.

[Octachron]

I have updated the Changes entry for the PR, and I am planning to merge it tomorrow if there are no objections.

[gasche]

I was about to do the same, so, yeah.

[Bannerets]

A pretty frequent mistake among OCaml beginners that I've seen is writing '\0' instead of '\000', I thought this PR should fix that. Would be very nice if the warning also included something about leading zeros in case of a numeric escape sequence, or maybe even better, special cased \0 with a suggestion of changing it to \000. (Also would be nice if the errors in chars displayed these hints too.) Perhaps this can be done in a different PR?

[Octachron]

Reading the error message in the test suite, I did think that we could go one step further and recognize that either \[0-9], \o[0-9][0-9]?, or \u{} are probably intended to be escape sequences. But I would recommend to go one step at a time and try that improvement in a subsequent PR.