Skip to content

Fix the display of utf8 characters in the code editor #412

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

Merged
merged 4 commits into from
Sep 15, 2021

Conversation

AltGr
Copy link
Collaborator

@AltGr AltGr commented Aug 11, 2021

Work done by @baloone during his internship at OCamlPro. This fixes the display with invalid syntax (display in strings and comments was already OK), but it shouldn't get scrambled either way.

NOTE: should be rebased and squashed once the 4.12 PR is merged

@erikmd erikmd added the needs: merge of dependency This PR depends on another PR being merged first. label Aug 25, 2021
@erikmd erikmd removed the needs: merge of dependency This PR depends on another PR being merged first. label Sep 15, 2021
@AltGr
Copy link
Collaborator Author

AltGr commented Sep 15, 2021

Rebased

@erikmd
Copy link
Member

erikmd commented Sep 15, 2021

Thanks a lot @AltGr ! I was precisely about to ask this to you 🙂

@erikmd
Copy link
Member

erikmd commented Sep 15, 2021

(Let's just wait for the CI though :)

@erikmd erikmd self-assigned this Sep 15, 2021
Copy link
Member

@erikmd erikmd left a comment

Choose a reason for hiding this comment

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

Hi again!
so the CI if fine, the code is tidy, and to experiment with some UTF-8 chars a bit (given this display issue was definitely something my students were stumbling on) I've Docker-built the PR branch (§) to get this toy example:

2021-09-15_19-38-35_Screenshot_UTF-8_ok

(§) docker build -t test-412 . && docker run --rm -it -p 8080:8080 -v "$PWD/demo-repository:/repository" test-412

so thanks a lot for this enhancement, @baloone and @AltGr 👍

(As an aside, I've did more tests with a fresh opam switch as well, and noticed a small issue for which I've found a workaround after investigation, but which happens to be orthogonal to this PR. So, merging — and opening another ticket subsequently.)

@erikmd erikmd merged commit debb635 into ocaml-sf:master Sep 15, 2021
@erikmd erikmd added kind: enhancement Enhancement to an existing user-facing feature. kind: fix labels Sep 15, 2021
@erikmd erikmd added this to the learn-ocaml 0.13 milestone Sep 15, 2021
@baloone
Copy link
Contributor

baloone commented Sep 22, 2021

Hello @erikmd, Thanks for the merge!

I think using smiley inside string has always worked fine, this fixes the display of Unicode characters outside of comments or string i.e. when the ocaml lexer throws an "illegal character" error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement Enhancement to an existing user-facing feature. kind: fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants