Skip to content
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

Use ASCII chars for MAM loading string #1820

Closed
wants to merge 1 commit into from
Closed

Use ASCII chars for MAM loading string #1820

wants to merge 1 commit into from

Conversation

alexandre1985
Copy link
Contributor

  • Fix non-ASCII chars, for correct display in TTY
  • UTF-8 personal names are excluded from this fix

@jubalh
Copy link
Member

jubalh commented Apr 5, 2023

Illegal chars? What's illegal about them?
I guess what you want to say is Use only ASCII characters in MAM loading string.

This change was introduced by @mdosch in #1773.

I can see a point in only using ASCII chars but I can also see a point in being a bit modern.

Maybe an alternative would be to configure your TTY: https://askubuntu.com/questions/23610/how-to-enable-unicode-support-in-a-tty

@alexandre1985
Copy link
Contributor Author

alexandre1985 commented Apr 5, 2023

@jubalh I use GNU Screen in my TTY and it doesn't show this chars correctly.
I have tried quitting GNU Screen, run unicode_start and still my TTY doesn't show those "illegal chars" correctly.

My system is Artix Linux and my TTY have lat9w-16 for the console font and 8859-1_to_uni as the console translation. My unicodemap is iso01. Is something missing to make the TTY more modern?

Maybe I should rename the commit. Asking suggestion for another name in place of "illegal chars".

@alexandre1985
Copy link
Contributor Author

alexandre1985 commented Apr 5, 2023

@jubalh I did check all files in the source code for "illegal chars", so that is why I am not referencing a single string or file.

One personal names in the copyright top comment of some files, was also being detected, but since it is a comment and since it is a personal name, I did not change it.

Also, besides that name being non-ASCII it renders well on my TTY.

@jubalh
Copy link
Member

jubalh commented Apr 5, 2023

I did check all files in the source code for "illegal chars", so that is why I am not referencing a single string or file.

In this kind of commit description how you got to this end result doesn't matter much. What you do change in this commit matters. And UTF-8 is nothing illegal ;)

One personal names in the copyright top comment of some files, was also being detected, but since it is a comment and since it is a personal name, I did not change it.

This and the fact that we use UTF-8 encoding for source files anyways.

Commit is now:

Fix char in MAM string to view it in TTY

 * Fix chars, for correct display in TTY. All source code files have
 been checked.
 * UTF-8 personal names are excluded from this fix.

And again I'm confused about the description. It's not a "fix".

@alexandre1985
Copy link
Contributor Author

@jubalh Ok, UTF-8 is not illegal. My phrasing could have been better. Please check now if it is acceptable.

@alexandre1985
Copy link
Contributor Author

And again I'm confused about the description. It's not a "fix".

@jubalh It would be better if you tell me what it is (instead of telling what is not). So that I may write a better commit message.

@jubalh
Copy link
Member

jubalh commented Apr 5, 2023

It would be better if you tell me what it is (instead of telling what is not). So that I may write a better commit message.

You mean like in my first comment #1820 (comment) where I wrote I guess what you want to say is **Use only ASCII characters in MAM loading string** ?

 * MAM loading string char to ASCII, for better display in all TTYs.
 * All source code files have been checked, and there is no other
 issue for TTYs chars display.

Signed-off-by: Daniel Santos <dacs.git@brilhante.top>
@alexandre1985
Copy link
Contributor Author

@jubalh is the new force push better?

@mdosch
Copy link
Contributor

mdosch commented Apr 5, 2023

As I already said in the MUC: is not an illegal character and it works fine in my TTY. I guess you have either some misconfiguration or you use a font which can't show the character . I think you should rather fix your TTY to also show non-ASCII characters than patching this out.
To check/change your setup do the following in Debian (I don't know how to do this on other distros):

# dpkg-reconfigure console-setup

@alexandre1985
Copy link
Contributor Author

Terminus shows the ellipsis char correctly in my terminal.

@mdosch So, you prefer to enforce a terminal font in all profanity users, than making this little adjustment?

I actually dislike terminus font look.

@mdosch
Copy link
Contributor

mdosch commented Apr 5, 2023 via email

@alexandre1985
Copy link
Contributor Author

alexandre1985 commented Apr 5, 2023

I want my font to be lat9w-16. And profanity does not display well that font.

Such a simple fix, there are many other '...' (3 dots) in the code. Why this resistance?

@alexandre1985
Copy link
Contributor Author

alexandre1985 commented Apr 5, 2023

You don't have to settle with Terminus, there are many more fonts which should support UTF8, so it's not like Terminus is enforced on all users. Also I think limiting characters to ASCII because people might use a font which doesn't support UTF8 is not a good solution. Anyway, I am not the one to decide…

When typing "in am not the one to decide..." why didn't you use the ellipsis char, instead of the 3 dots?

Would be awesome to know, and this might lead somewhere...

@mdosch
Copy link
Contributor

mdosch commented Apr 5, 2023 via email

@alexandre1985
Copy link
Contributor Author

On 05.04.2023 12:08, Daniel Santos wrote: When typing "in am not the one to decide..." why didn't you use the ellipsis char, instead of the 3 dots?
That's exactly what I did… rofl

Yes, I did a mistake.

@alexandre1985
Copy link
Contributor Author

alexandre1985 commented Apr 5, 2023

I just counted, the 3 dots are printed 26 times by profanity. The ellipsis char is printed 1 time by profanity. Choose one. @mdosch

@alexandre1985
Copy link
Contributor Author

alexandre1985 commented Apr 6, 2023

Here is my case:

In order for this project be consistent to the user, I am considering that for strings printed to the user, the project will either use 3dots or the ellipsis char for all cases.

=== 3dots ===

** Advantages **

  1. Works in all TTY displays, independent of TTY font.
  2. User is free to choose whatever font they wish for their TTY.
  3. Needs to change 1 entry of commited work from 1 developer, instead of 26 entries from numerous developers.

** Disavantages **

None.

=== Ellipsis char ===

** Advantages **

  1. Use a new char.

** Disavantages **

  1. Doesn't work in all TTY displays, independent of TTY font.
  2. Forcing user to use a small group of today's TTY fonts.
  3. Needs to change 26 entries of commited work from numerous developers, instead of 1 entry from one developer.
  4. The new ellipsis char has always been replaced by 3dots chars, before the ellipsis char existence. It is redundant.

=====

If it is chosen to not do a thing, consistency is lost and is gained Ellipsis Disavantage 1. and 2., Ellipsis advantage 1. and not doing any commit.

Choose wisely.

@jubalh jubalh changed the title Fix illegal chars Use ASCII chars for MAM loading string Apr 8, 2023
@jubalh
Copy link
Member

jubalh commented Apr 8, 2023

I updated the PR title for you, please do this the next time as well.

I checked on both my Gentoo and openSUSE machines. On both Unicode on TTY works without problems. All with default settings, I didn't change/configure anything in this regard manually.

@jubalh jubalh closed this Apr 8, 2023
@alexandre1985
Copy link
Contributor Author

Such a simple to approve PR. A no-brainer.

You have definitely not tried on Arch Linux. It does not work in Arch. Arch Linux has a much bigger number of users than either Gentoo or OpenSUSE.

And I guess you don't want consistency in on Profanity. What a poor decision.

I updated the PR title for you, please do this the next time as well.

There is no need, there won't be a next PR from me.

@sjaeckel
Copy link
Member

sjaeckel commented Apr 9, 2023

btw. I'm using Arch as well, and it works for me

@alexandre1985
Copy link
Contributor Author

btw. I'm using Arch as well, and it works for me

It does not work on the TTY with the default font (lat1-16 or lat9w-16).

@mdosch
Copy link
Contributor

mdosch commented Apr 9, 2023 via email

@mdosch mdosch mentioned this pull request Apr 9, 2023
mdosch added a commit to mdosch/profanity that referenced this pull request Apr 9, 2023
As stated in
profanity-im#1820 (comment)
profanity uses "..." (three dots) in a lot of places instead the proper
ellipsis char "…".
@jubalh jubalh added the abuse label Apr 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants