Big5 support by pkerichang · Pull Request #1808 · Mudlet/Mudlet

pkerichang · 2018-07-12T21:28:10Z

Brief overview of PR changes/additions

This is a re-submit of the big5 encoding support pull request, with a more descriptive branch name.

2. added range check on the second byte. 3. do not set isNonBmpCharacter to true.

SlySven

This looks good to go. I trust that the whole thing is working now we reverted that other PR that broke things...

Realised that a further change was neeed - will reappove when that is done or agreed to be unnecessary

SlySven · 2018-07-14T01:21:09Z

I have managed to make a connection to that MUD that you used and it seems to work, but will leave squashing and merging to @vadi2 depending on whether he wants to drop it into the next release or hold it to the one afterwards:

Ah - there is one question that we need to address before merging and that is "what width setting should East Asian ambiguous width characters" be drawn with by default for this encoding - I would tend to think that it is likely to need to be "Wide" for this encoding but could you @pkerichang try setting the option to both the narrow (unchecked) and wide (checked) setting and tell us which the default (partially checked) setting should default to when "Big5" is selected?

I think such characters need to be drawn as wide as by default so this encoding (besides GBK and GB18030) should use the wide setting - to do this can you edit the above tool-tip which is set in the dlgProfilePreferences constructor:

    checkBox_useWideAmbiguousEastAsianGlyphs->setToolTip(QStringLiteral("<html><head/><body>%1</body></html>")
                                                         .arg("<p>Some East Asian MUDs may use glyphs (characters) that Unicode classifies as being "
                                                              "of <i>Ambigous</i> width when drawn in a font with a so-called <i>fixed</i> pitch; in "
                                                              "fact such text is <i>duo-spaced</i> when not using a proportional font. These symbols can be "
                                                              "drawn using either a half or the whole space of a full character. By default Mudlet tries to "
                                                              "chose the right width automatically but you can override the setting for each profile.</p>"
                                                              "<p>This control has three settings:"
                                                              "<ul><li><b>Unchecked</b> '<i>narrow</i>' = Draw ambiguous width characters in a single 'space'.</li>"
                                                              "<li><b>Checked</b> '<i>wide</i>' = Draw ambiguous width characters two 'spaces' wide.</li>"
                                                              "<li><b>Partly checked</b> <i>(Default) 'auto'</i> = Use 'wide' setting for MUD Server "
                                                              "encodings of <b>GBK</b> or <b>GBK18030</b> and 'narrow' for all others.</li></ul></p>"
                                                              "<p><i>This is a temporary arrangement and will likely to change when Mudlet gains "
                                                              "full support for languages other than English.</i></p>"));

by changing the line:

"encodings of <b>GBK</b> or <b>GBK18030</b> and 'narrow' for all others.</li></ul></p>"

to:

"encodings of <b>Big5</b>, <b>GBK</b> or <b>GBK18030</b> and 'narrow' for all others.</li></ul></p>"

If you can also change:

 "<p><i>This is a temporary arrangement and will likely to change when Mudlet gains "

to:

 "<p><i>This is a temporary arrangement and will be likely to change when Mudlet gains "

that will also fix a typo that I have just spotted! 😀

To actually make the effect take place in code, go to Host.cpp and change the following method at the indicated place:

 void Host::setWideAmbiguousEAsianGlyphs(const Qt::CheckState state)
 {
     bool localState = false;
     bool needToEmit = false;
     const QString encoding(mTelnet.getEncoding());

     QMutexLocker locker(& mLock);
     if (state == Qt::PartiallyChecked) {
         // Set things automatically
         mAutoAmbigousWidthGlyphsSetting = true;

-        if ( encoding == QLatin1String("GBK")
+        if ( encoding == QLatin1String("Big5")
+           || encoding == QLatin1String("GBK")
            || encoding == QLatin1String("GB18030")) {

             // Need to use wide width for ambiguous characters
             if (!mWideAmbigousWidthGlyphs) {
                 // But the last setting was narrow - so we need to change
                 mWideAmbigousWidthGlyphs = true;
                 localState = true;
                 needToEmit = true;
             }

         } else {
             // Need to use narrow width for ambiguous characters
             if (mWideAmbigousWidthGlyphs) {
                 // But the last setting was wide - so we need to change
                 mWideAmbigousWidthGlyphs = false;
                 localState = false;
                 needToEmit = true;
             }

         }

     } else {
         // Set things manually:
         mAutoAmbigousWidthGlyphsSetting = false;
         if (mWideAmbigousWidthGlyphs != (state == Qt::Checked)) {
             // The last setting is the opposite to what we want:

             mWideAmbigousWidthGlyphs = (state == Qt::Checked);
             localState = (state == Qt::Checked);
             needToEmit = true;
         };

     }

     locker.unlock();
     // We do not need to keep the mutex any longer as we have a local copy to
     // work with whilst the connected methods react to the signal:
     if (needToEmit) {
         emit signal_changeIsAmbigousWidthGlyphsToBeWide(localState);
     }
 }

OTOH If you do not think that this change is needed can you confirm this - then these changes will not be needed. In case you do not understand it I will point you to https://www.unicode.org/reports/tr11/tr11-35.html .

pkerichang · 2018-07-14T02:42:40Z

Hello,

I do think it is a good idea to change default width of ambiguous characters. I've committed the changes.

vadi2 · 2018-07-14T06:18:47Z

@pkerichang would it be OK for this to wait for the next release (in a month or sooner)? We're releasing tomorrow and are in the new features freeze now.

pkerichang · 2018-07-14T06:57:59Z

I'm fine with that. Thanks for letting me know.

SlySven

👍 Okay

vadi2 · 2018-07-16T12:22:14Z

@pkerichang it actually made it into 3.11.1, fyi

# By Vadim Peretokin (81) and others * development: (162 commits) BugFix: set Server Encoding correctly on auto-loading profiles Install xz-utils to guarantee xz is available Create and upload source tarballs on release BugFix: use Alternative OpenSSLBinary for Windows (Mudlet#1850) Fix auto-save kicking in and blocking save profile as Upgrade a few classes to newer Qt connect style (Mudlet#1846) Fix package exporter to work with async save (Mudlet#1832) Pulled out actions into a mudlet member class variables (Mudlet#1839) Create module zip if it's not already created when syncing (Mudlet#1842) i18n-ise GUI label creation (Mudlet#1838) Align "no map"message in centre propely (Mudlet#1837) New Crowdin translations (Mudlet#1756) Delete old version checks (Mudlet#1833) Re-set dev. 3.11.1 bugfix release. Fix iterator to actually iterate BugFix: fix faulty log options for new profiles or old profile save files Big5 support (Mudlet#1808) BugFix: include a fail-back icon for the auto-saved profile in Con. Dialog Back to development we go! ... Conflicts resolved in: * src/TTextEdit.cpp * src/TTextEdit.h Signed-off-by: Stephen Lyons <slysven@virginmedia.com>

# By Vadim Peretokin (24) and others # Via keneanung * development: (52 commits) Install xz-utils to guarantee xz is available Create and upload source tarballs on release BugFix: use Alternative OpenSSLBinary for Windows (Mudlet#1850) Fix auto-save kicking in and blocking save profile as Upgrade a few classes to newer Qt connect style (Mudlet#1846) Fix package exporter to work with async save (Mudlet#1832) Pulled out actions into a mudlet member class variables (Mudlet#1839) Create module zip if it's not already created when syncing (Mudlet#1842) i18n-ise GUI label creation (Mudlet#1838) Align "no map"message in centre propely (Mudlet#1837) New Crowdin translations (Mudlet#1756) Delete old version checks (Mudlet#1833) Re-set dev. 3.11.1 bugfix release. Fix iterator to actually iterate BugFix: fix faulty log options for new profiles or old profile save files Big5 support (Mudlet#1808) BugFix: include a fail-back icon for the auto-saved profile in Con. Dialog Back to development we go! 3.11.0 release ... Conflicts resolved in: * src/dlgProfilePreferences.cpp * src/mudlet.qrc Signed-off-by: Stephen Lyons <slysven@virginmedia.com>

pkerichang added 5 commits July 1, 2018 12:40

Adding support for Big5 encoding

424e3de

1. clean up comments.

85cbf0d

2. added range check on the second byte. 3. do not set isNonBmpCharacter to true.

Merge branch 'qt57' into last_working_merge1

af2302d

merging big5 support to current development branch.

fd0d13d

update status flags if Qt decoder fail to parse some characters.

05e46b6

pkerichang requested a review from a team as a code owner July 12, 2018 21:28

pkerichang requested a review from a team July 12, 2018 21:28

SlySven previously approved these changes Jul 13, 2018

View reviewed changes

change default width of ambiguous characters when using big5 encoding.

9dd0a62

SlySven approved these changes Jul 14, 2018

View reviewed changes

SlySven added the 3 - Done (Merge hold) label Jul 14, 2018

SlySven assigned vadi2 Jul 14, 2018

SlySven added this to the 3.12 milestone Jul 14, 2018

vadi2 merged commit 8161ea8 into Mudlet:development Jul 15, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Big5 support#1808

Big5 support#1808
vadi2 merged 6 commits intoMudlet:developmentfrom
pkerichang:big5-support

pkerichang commented Jul 12, 2018

Uh oh!

SlySven left a comment

Uh oh!

SlySven commented Jul 14, 2018

Uh oh!

pkerichang commented Jul 14, 2018

Uh oh!

vadi2 commented Jul 14, 2018

Uh oh!

pkerichang commented Jul 14, 2018

Uh oh!

SlySven left a comment

Uh oh!

vadi2 commented Jul 16, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

pkerichang commented Jul 12, 2018

Brief overview of PR changes/additions

Uh oh!

SlySven left a comment

Choose a reason for hiding this comment

Uh oh!

SlySven commented Jul 14, 2018

Uh oh!

pkerichang commented Jul 14, 2018

Uh oh!

vadi2 commented Jul 14, 2018

Uh oh!

pkerichang commented Jul 14, 2018

Uh oh!

SlySven left a comment

Choose a reason for hiding this comment

Uh oh!

vadi2 commented Jul 16, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants