Skip to content

Delete non-ASCII characters from header #3588

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 1 commit into from
Jun 20, 2023

Conversation

jwpeterson
Copy link
Member

This is kind of a real fix, but also a test of whether CIVET is working again.

@roystgnr
Copy link
Member

So weird. Personally I have to hold myself back from going crazy with UTF-8 in comments, d∈ℝ³ and so forth, but where'd we get non-ASCII whitespace?

@dschwen
Copy link
Member

dschwen commented Jun 20, 2023

Why are we still so picky about unicode? Don't all compilers support it in the source code by now? Are we worried about copy pasting invisible characters that could lead to all kinds of confusion? It sure would be nice to use greek letters in comments with equations (or even in identifier names!)

@jwpeterson
Copy link
Member Author

Why are we still so picky about unicode?

In this case, the use of non-ASCII characters wasn't intentional, so it made sense to remove it. I'm not aware if we have a libMesh rule explicitly allowing or disallowing unicode characters either in comments or in actual code? Personally I think it's a little gimmicky and probably wouldn't use them much, but if people really want to do it, I guess it would be fine, maybe after one more ASCII-only release?

@moosebuild
Copy link

Job Distributed make check sweep (even) on 1cfb948 : invalidated by @jwpeterson

Retry after fixing CIVET

@moosebuild
Copy link

Job Distributed make check sweep (odd) on 1cfb948 : invalidated by @jwpeterson

Retry after fixing CIVET

@moosebuild
Copy link

Job Min clang on 1cfb948 : invalidated by @jwpeterson

Retry after fixing CIVET

@moosebuild
Copy link

Job Min gcc on 1cfb948 : invalidated by @jwpeterson

Retry after fixing CIVET

@moosebuild
Copy link

Job Test 32bit on 1cfb948 : invalidated by @jwpeterson

Retry after fixing CIVET

@moosebuild
Copy link

Job Test MOOSE clang on 1cfb948 : invalidated by @jwpeterson

Retry after fixing CIVET

@roystgnr
Copy link
Member

https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2295r6.pdf

Clang basically assumes everything is UTF-8, but GCC and MSVC require a special flag or environment, and other compilers ... were apparently not worth mentioning directly? But it looks like the "Edison Design Group" did the frontend for Portland Group compilers, from which NVidia's HPC stuff is descended, so maybe nvc++ is solid?

I love writing with Unicode, but I'd just as soon wait until Unicode support is in an official standard, not just a common extension. I'd also worry a little bit about users' editor support, especially if we want to use unicode in identifiers. XCompose is what makes me happy to write with Unicode, but last I looked you have to get third party programs to make that work on Windows or Mac, and even on Linux I've been maintaining my own .XCompose file to hit all the fun math characters that don't have compose sequences by default. In the default Compose file, ironically, ¬∃ !

@moosebuild
Copy link

Job Test MOOSE debug on 1cfb948 : invalidated by @jwpeterson

Retry after fixing CIVET

@moosebuild
Copy link

Job Test MOOSE min clang on 1cfb948 : invalidated by @jwpeterson

Retry after fixing CIVET

@moosebuild
Copy link

Job Test MOOSE min gcc on 1cfb948 : invalidated by @jwpeterson

Retry after fixing CIVET

@moosebuild
Copy link

Job Test clang on 1cfb948 : invalidated by @jwpeterson

Retry after fixing CIVET

@moosebuild
Copy link

Job Test complex and infinite on 1cfb948 : invalidated by @jwpeterson

Retry after fixing CIVET

@moosebuild
Copy link

Job Test debug on 1cfb948 : invalidated by @jwpeterson

Retry after fixing CIVET

@roystgnr
Copy link
Member

I'd also worry a little bit about visual conflicts. Does ι clearly look like an iota in everybody's editor? If I'm using α and Β as identifiers, are you going to try to reference B or Β later? They look indistinguishable to me in both my browser and my terminal, though one is ASCII "bee" (0x42) and the other is Greek "beta" (U+0392, 0xCE92).

@moosebuild
Copy link

Job Test with threads on 1cfb948 : invalidated by @jwpeterson

Retry after fixing CIVET

@moosebuild
Copy link

Job Test mac on 1cfb948 : invalidated by @jwpeterson

Retry after fixing CIVET

@moosebuild
Copy link

Job Test debug on 1cfb948 : invalidated by @jwpeterson

Retry after fixing CIVET

@moosebuild
Copy link

Job Test complex and infinite on 1cfb948 : invalidated by @jwpeterson

Retry after fixing CIVET

@moosebuild
Copy link

Job Test clang on 1cfb948 : invalidated by @jwpeterson

Retry after fixing CIVET

@roystgnr
Copy link
Member

https://civet.inl.gov/job/1598122/ is still failing, but at least this time the bug turns out to be "clang++-12 doesn't support -fsanitize-ignorelist like clang++-14 did", not "Roy can't write bash correctly" any more.

@moosebuild
Copy link

Job Coverage on 1cfb948 wanted to post the following:

Coverage

No coverage report(s) were found for the base commit 2a70c84.
Full coverage report

This comment will be updated on new commits.

@jwpeterson jwpeterson merged commit bc2b494 into libMesh:devel Jun 20, 2023
@jwpeterson jwpeterson deleted the fix_non_ascii branch June 21, 2023 17:09
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.

4 participants