Give meaning to encodings by vadi2 · Pull Request #1107 · Mudlet/Mudlet

vadi2 · 2017-06-19T20:17:27Z

Turns an unintelligible list of technical standards:

Into something friendlier:

Credit to encoding data from https://github.com/gnunn1/tilix/blob/master/source/gx/tilix/encoding.d.

…o-encodings

SlySven

Qt does already have aliases for some of the encodings and I only choose one of them as an identifier, reviewing the Qt source code may reveal a better friendly name!

Be advised that I do have a prototype GB2312/GBK{a superset of GB2312 using one/two bytes per Unicode BMP-codepoint} & GB18030 {a superset of the others using one/two or four bytes per possibly non-BMP codepoint} decoder also awaiting PRing - that may complicate things as those two are also handled separately in a sub-method (I also hived off the UTF-8 stuff to new sub-method)...

SlySven · 2017-06-20T03:07:21Z

src/TBuffer.cpp

@@ -1922,7 +1955,7 @@ void TBuffer::translateToPlainText( std::string & incoming, const bool isFromSer
                            }
                            pRef += 5;

-                            QChar _quote_type = _t2[pRef];
+               QChar _quote_type = _t2[pRef];


Dunno. Fixed.

SlySven · 2017-06-20T03:08:46Z

src/TBuffer.cpp

+    auto& smallestkey = csmEncodingTable.firstKey();
+    qWarning() << "TBuffer::getComputerEncoding:" << encoding << "not found, returning" << smallestkey;
+    return smallestkey;
+}


EOF EOL missing!

☑️ Okay.

SlySven · 2017-06-20T03:11:56Z

src/TBuffer.cpp

-      QChar(0x00F8), QChar(0x00B9), QChar(0x0157), QChar(0x00BB), QChar(0x00BC), QChar(0x00BD), QChar(0x00BE), QChar(0x00E6),  // B8-BF
-      QChar(0x0104), QChar(0x012E), QChar(0x0100), QChar(0x0106), QChar(0x00C4), QChar(0x00C5), QChar(0x0118), QChar(0x0112),  // C0-C7
-      QChar(0x010C), QChar(0x00C9), QChar(0x0179), QChar(0x0116), QChar(0x0122), QChar(0x0136), QChar(0x012A), QChar(0x013B),  // C8-CF
-      QChar(0


🌍 I18n Uh-oh - these texts are very definitely something that will be wanted to be translated - so please swap the QStringLiterals( for tr(

Oh dear, that comment slipped out of place - it was referring to the friendly name declaration for each table.

SlySven · 2017-06-20T03:15:35Z

src/TBuffer.h


 class TBuffer
 {
-    static const QMap<QString, QVector<QChar>> csmEncodingTable; // private!


You will need to add:

Q_DECLARE_TR_FUNCTIONS(TBuffer) // Needed so we can use tr() even though TBuffer is NOT derived from QObject

here at the start of the class declaration, so you can use the tr(...) wrapper to mark (GUI) translatable text...!

Thanks, that's exactly what I needed.

Well, not exactly, it does not allow passing disambiguation text to the translator, and while Qt documentation says this is no longer the preferred way of doing so, it fails to provide the modern way.

Not entirely sure what you are saying here about the disambiguation text unless you mean that QTBUG-43540 is also factor here. Which is a pity, as it means ensuring that special marked C/C++ code comments are needed:

/*:string*/ or //:string to replace the "disambiguation" string as a second argument totr

//=<id> to provide an "id" based unique identifier string for this instance of this QString for id based translators (but probably NOT Qt Liguist!)

//~<field> <value> to provide an XML marked up <extra-field>value</extra-field> additional extra comment that is included in the .ts file (but, as part of the above bug) totally hidden/ignored by Qt Linguist

Of course those comments could get detached or mangled because they are not "part" of the tr(...) call but that is the Qt people's decision and the rest of us just have to live with it... 😒

☑️ I guess this bit is Okay now...

vadi2 · 2017-06-20T22:14:20Z

@SlySven updated, have a look again.

itsTheFae

One step closer to i18n 😄

SlySven · 2017-06-27T01:07:47Z

src/TBuffer.cpp

-      QChar(0x00F8), QChar(0x00B9), QChar(0x0157), QChar(0x00BB), QChar(0x00BC), QChar(0x00BD), QChar(0x00BE), QChar(0x00E6),  // B8-BF
-      QChar(0x0104), QChar(0x012E), QChar(0x0100), QChar(0x0106), QChar(0x00C4), QChar(0x00C5), QChar(0x0118), QChar(0x0112),  // C0-C7
-      QChar(0x010C), QChar(0x00C9), QChar(0x0179), QChar(0x0116), QChar(0x0122), QChar(0x0136), QChar(0x012A), QChar(0x013B),  // C8-CF
-      QChar(0


Do we have any idea who uses "-11" ah, it is Latin/Thai according to Wikipedia - but unlike other 8859 encoding it is NOT an IANA registered encoding and it include what looks like combining characters (though not diacriticals) at:

D1

D4 to DA

E7 to EE

Those may prove to need a different decoding system...

OK, added that description.

SlySven · 2017-06-27T01:11:17Z

src/TBuffer.cpp

-      QChar(0x00F8), QChar(0x00B9), QChar(0x0157), QChar(0x00BB), QChar(0x00BC), QChar(0x00BD), QChar(0x00BE), QChar(0x00E6),  // B8-BF
-      QChar(0x0104), QChar(0x012E), QChar(0x0100), QChar(0x0106), QChar(0x00C4), QChar(0x00C5), QChar(0x0118), QChar(0x0112),  // C0-C7
-      QChar(0x010C), QChar(0x00C9), QChar(0x0179), QChar(0x0116), QChar(0x0122), QChar(0x0136), QChar(0x012A), QChar(0x013B),  // C8-CF
-      QChar(0


Apparently CP850 is a Western Europe encoding, Wikipedia notes:

"Code page 850 differs from code page 437 in that many of the box drawing characters, Greek letters, and various symbols were replaced with additional Latin letters with diacritics, thus greatly improving support for Western European languages (all characters from ISO 8859-1 are included). At the same time, the changes frequently caused display glitches with programs that made use of the box-drawing characters to display a GUI-like surface in text mode."

SlySven · 2017-06-27T01:53:51Z

src/TBuffer.cpp

@@ -4149,3 +4182,42 @@ QString TBuffer::bufferToHtml( QPoint P1, QPoint P2, bool allowedTimestamps, int

    return s;
 }
+
+const QList<QString> TBuffer::getComputerEncodingNames() {


This seems a bit more complicated than it needs to be, can it not be inlined in the header file as:

const QList<QString> getComputerEncodingNames() const {return csmEncodingTable.keys()};

or given that it is a const static value perhaps things are more complicated so perhaps it would need to stay in this file, but it does seem strange that you are creating an automatic/local QList<QString> instance to return data that is unchanging and static...

const QList<QString> getComputerEncodingNames() const {return TBuffer::csmEncodingTable.keys() }

Hell, it might even be possible to return a const reference... 😖

Fixed. I made the friendly one first which is why this one came out like that - thanks for spotting an improvement!

SlySven · 2017-06-27T02:00:32Z

src/TBuffer.cpp

+}
+
+const QList<QString> TBuffer::getFriendlyEncodingNames() {
+    QList<QString> encodings;


Compared to the previous method (getgetComputerEncodingNames) I agree that you will likely have to iterate through the csmEncodingTable.values() because I cannot see any short-cut provided by Qt to get just all the first or second out of a "container" data-type with a QPair<T1,T2> value type...

SlySven · 2017-06-27T02:06:27Z

src/TBuffer.h

+
+    // private - a map of computer-friendly encoding names as keys,
+    // values are a pair of human-friendly name + encoding data
+    static const QMap<QString, QPair<QString, QVector<QChar>>> csmEncodingTable;


💭 Although bundling the encoding data and the "friendly" name as a QPair encapsulates the data nicely, from a coding point of view it may be simpler to have two separate QMaps...?

I am going for the nice data encapsulation, yep.

vadi2 · 2017-06-27T12:46:22Z

@SlySven feedback addressed. I'm hoping this'll get approved soon so I can make another PR that fixes the ordering not to go from 10-15 followed by 2-9.

…o-encodings # Conflicts: # src/TBuffer.cpp # src/ctelnet.cpp # src/dlgProfilePreferences.cpp

SlySven

Seems OK but I wonder about a couple of method return types!

SlySven · 2017-06-28T15:32:48Z

src/TBuffer.h

    void paste(QPoint&, TBuffer);
    void setBufferSize(int s, int batch);
-    static const QList<QString> getComputerEncodingNames();
+    static const QList<QString> getComputerEncodingNames() { return csmEncodingTable.keys(); };


😕 Is it really both static and const - normally a const on a function return is useless unless it is a reference that is being passed and QMap<T1,T2>::keys() returns QList<T1> as a value not a reference. Though csmEncodingTable is a static, const item...

I don't know. Doesn't seem to hurt though.

SlySven · 2017-06-28T15:38:01Z

src/TBuffer.h

-    static const QList<QString> getHardCodedEncodingTableKeys() {return csmEncodingTable.keys();}
+    static const QList<QString> getComputerEncodingNames() { return csmEncodingTable.keys(); };
+    static const QList<QString> getFriendlyEncodingNames();
+    static const QString& getComputerEncoding(const QString& encoding);


Are these static const though - const is normally ineffective on returns from methods/functions unless it is a reference that is being returned (otherwise it is a value or pointer and constentness cannot be enforced or is irrelevant) IIRC.

I've just followed what you had above with getHardCodedEncodingTableKeys()!

* Removed duplicate clear event handlers call * Revert back to dev version. 3.3 here we go! * Adds saveWindowLayout() call to AutoSaveOnExit processing. * Edbee editor implementation (#985) * Added a frame to edbee so it looks nicer * Got utf8 module to load (#1074) (update packaging!) * Modified tooltip for Save and Save Profile to clearly indicate their keyboard shortcuts * Added: shortcut for delete action in editor (#1084) * shortcut added for delete action * Removed redundant edbee config related code (#1091) * Delete TScriptEditorManager.h * Delete TScriptEditorManager.cpp * BugFix: import some fixes by "Zolder" from Mudlet (Realms of Legends) fork import: some Zoilder fixes from the MudletRL fork: * in TBuffer::translateToPlainText(...): make some MXP related code execute only if Host::mFORCE_MXP_NEGOTIATION_OFF is NOT set as well as another condition. * in TBuffer::wrap(...): change a for(...) {...} loop termination condition be a "<" rather than a "<=" test - which sounds like an "off-by-one" error fix-up. * In TTextEdit:updateScreenView(): revise code that seems to prevent the width of a "main" console being set too small and NOT written to the "parent" Host::mScreenWidth variable - the original modification removed such a restriction completely, but I have just lowered the minimum from 100 to 40 and ensured it is always stored both in the local "main" console and to the parent record in the Host class for that console. * In TTextEdit::drawForeground(...): revise the behaviour *I think* when at the point where a console "split" becomes unnecessary... Signed-off-by: Stephen Lyons <slysven@virginmedia.com> * Cut down on the length of some variable names (#1086) * Clarified use of QLatin1String * Adding GCC 5 for C++14 requirement (#1098) * Fixed "reset map colors" button to work (#1049) * Fixed "reset map colors" button to work That's a 5 year old bug out of the way! * Change travis configuration to use installed gcc-5 (#1108) * Change travis configuration to use installed gcc-5 * Fixes #1104 (#1109) Thanks to Rick for his prompt response on this * Fixed typo in GeyserLabel.lua Per #1112 * Fix locale to C Enforce uniform locale so scripts don't get tripped up on number representation differences (. vs ,) * IRC client update of UI and logic (#1072) * Add Communi Model & Util libs * Full refactor of IRC Client code & UI This retains the primary functions of the old IRC client but adds many more new IRC commands by default. The UI is slightly more organized and interactive now. * Adds QPointer to IRC Dialog pointer variable. * Update Lua `sendIrc()` slightly. * Added /msg command, Input History, and updated display colors. Input History stores a total of 8 previously entered commands the user can cycle through using the Up Arrow key. The previously include /msg command has been added again. Colors of some messages have been updated for (relatively) easier distinction of message lines. * Updated channel handling. * Fix CMake builds Things needed to fix CMake builds: - communi CMake builds needed `include_directories` explicitly specified - more MOC calls in communi - more explicit dependencies to MOC files in communi - ircmessageformatter must be compiled into mudlet * IRC3 with more Lua features (#1087) Changes IRC client to run with settings given per-profile, making the IRC client more visible and configurable within Lua. This adds configuration options to the settings window in the "Special Options" tab for configuring an IRC client on the host profile which opens the settings window. See the PR summary comment for details on available Lua functions and IRC Client commands. * Add a Code of Conduct (#1110) * Create CODE_OF_CONDUCT.md * Fix for the QMake warnings caused by the IRC library includes. * Finished formatting all filed & added braces (#1115) * Finished formatting all filed & added braces * Re-enabled map load on profile load (#1118) * Re-enabled map load on profile load - disabling it broke scripts that relied on map data w/o the map open * updated assignments. * Event-based gamepad functionality (#1106) * Add theme switching to code editor (#1095) * Fix more Toolbar & Button bugs (#1080) (#1090) * Fix bugs with Buttons & Toolbars. - Fixed change detection for location, orienation, and CSS data. - Buttons are not rendered unless they are part of a group, Toolbar/Menu. - ToolBars whos locations are changed now hide their previous TToolBar or TEasyButtonBar instance as needed. * Fix Buttons being left behind when parent toolbar is deleted. Also allows TAction to use QPointer<T> * Update old sourceforge page links (#1132) (#1134) * Fixed blank command separator to not separate as people expect * Replaced isTempKey, isTempAlias, etc with isTemporary (#1102) * also pushed isFolder() and setIsFolder() to the parent Tree class * Give meaning to encodings (#1107) * Delete dev_README Deleted outdated dev_README file. It also didn't seem to be used all that often, else people would have raised the fact that it's outdated... * Update travis to use Qt 5.9 for compiling (#1141) * Re-add single QT 5.6 build job Since we want to keep sure that the minimal Qt version is still compilable (currently we need at least 5.6), we add a single job for that version. * Refactor: rename QMake project/sub-project files (#1103) * refactor: rename QMake project/sub-project files It is down right confusing having several project files in the Qt IDE which are all labelled "src" - this commit renames the main project file from "src.pro" to "mudlet.pro" and also renames: * the lua_yajl module from src.pri to lua_yajl.pri * the luazip module from src.pri to luazip.pri it also: * adds a new communi.pri file (see below) I also took the liberty of renaming the project resource file from: * mudlet_alpha.qrc to mudlet.qrc (we are well past the alpha stage now!) and sorting a couple of items into the right order. By creating a local project file 3rdparty/communi/communi.pri it is possible to group the three separate but related communi libraries into one - as it happens the effective content of the new file is identical to that of the communi libraries own libcommuni/src/src.pri but it has some notes on how the portion of the upstream library that we are using was picked out of there; how to identify the effect version and how to (hopefully) recreate things if upstream gets revised in the future! By not using the upstream "src.pri"/"src.pro" files we avoid the confusion of having a generic "src" project in the Mudlet project as a whole and it is clearly identify-able as a file from the Mudlet project rather than the sub-project that it pulls into the whole thing... There is an minor nuisance side-effect in that having the main project src.pro file open in a Qt Creator IDE or stored in a session will get confused when this commit takes effect as the file will disappear from the repository - it will be necessary to close the project and open the new src/mudlet.pro project file (and possibly re-enter the build steps) - OTOH the main project will now be identified as "mudlet" in Creator rather than the previous "src". 😄 Signed-off-by: Stephen Lyons <slysven@virginmedia.com> * Removed accidental debug echo left in (#1151) * initialize int nIrcPort with dlgIRC::DefaultHostPort value. (#1153) * Removes false from sendIrc() return, ignores command filtering status (#1154) * Removes false from sendIrc() return, ignores command filtering status (not used). * Remove punctuation from lua messages * Improved layout of IRC preferences (#1152) * Fix get map events (#1120) * Adapt addMapEvent error messages to current standard * Fix getMapEvents() The function now returns a proper indexed table of objects that each describe a map event. * Add missing call to set Editor as editable. (#1159) * Fixed tab indent to be two spaces again * Fixed getComputerEncoding in case no key found It'll now return the original value passed - it's way better than returning the wrong value. * Add linux ci deployment (#1128) * Modify files needed for CI deployment of linux builds * Syntax fix in .travis.yml * tar the CI AppImages as well * Add lua sqlite runtime dependency * Unset LD variable for linuxdeployqt * Bugfix: unset LD_LIBRARY_PATH in every codepath * Test: install appmenu-qt5 for global menus * Revert 781ef97 (Install appmenu-qt5) The issue was that the first AppImage used Qt 5.6.2 instead of Qt 5.9 (which is how 3.2 was built and how it is build now after merging development in) * Remove manager again (#1163) * Improved map load output to be condensed down if it's all successful (#1117) OK! Would be happy to see that in as well for maps that take a while to load (>1s). * Re-enable search highlighting (#1096) * Remove mainwindow statusbar (#1162) * Replaced statustips with tooltips in the mapper menu * Removed statusbar * Tagging 3.3.0 * Initial QTextToSpeech additions * Further TTS additions and bugfixes * Wrapping TTS functions in #ifdefs for QT_TTS_LIB * Missing #ifdefs * More misplaced #ifdef lines for TTS functions * More QT_TTS_LIB ifs * Adding QTextToSpeech to cmake lists * Various TTS updates * Adding GetState to TLuaInterpreter.h * Minor fixes * Tts temporary fixes * Exchanging toLatin1 with toUtf8 * Fix cmake TTS module includesion * Install Qt 5.9 TTS module on linux * Use Qt provided definition for enabled modules We don't need (and possibly don't want) to provide our own definition for when a module is enabled. Let's use the Qt provided definition instead. * Output confirmation messages of enabled Qt modules * Use upper case "Gamepad" in output * ttsClearQueue crash fix * Fix for global speech events * Whitespace replacement * Various minor fixes * More minor fixes. * Cleanup: make CMakeList.txt have uniform indenting and adjust CMake message The minimum versions of the Qt TextToSpeech and Gamepad reported in the CMake project files needed tweaking. I have also tried to regularise the indentation in the Mudlet CMake project files to standardise on 2 spaces. Signed-off-by: Stephen Lyons <slysven@virginmedia.com> * Reformat: clean up code-loyout of new TTS TLuaInterpreter code Signed-off-by: Stephen Lyons <slysven@virginmedia.com> * BugFix: fixup a switch() in (int) TLuaInterpreter::ttsGetState(lua_State*) This switch was missing "break;" on all the cases and must have been defective without them - for all but the last case extra strings would have been pushed onto the lua stack but as the return value was 1 they may have been discarded... Signed-off-by: Stephen Lyons <slysven@virginmedia.com> * Enable Qt 5.9 Windows builds for testing * Deploy Qt 5.9 builds for testing * Fixing double-events for queueing purposes * Added missing symbol * Added missing symbol * Updating tts events to globals and adding voice * Create TTSValues.lua * Adding TTSValues to LuaGlobal * Adding TTSValues to Mudlet project * Update CMakeLists.txt * Update TTSValues.lua * Update Qt to 5.10 for trusty This is needed to hopefully have some TTS backend plugins in Linux * Queueing and crash fixes * Renaming tts funcs * Renaming internal functions * Adding functions for retrieving settings * Fixing typo * Making ttsGetState public * Improve error messages for ttsGetCurrentLine() It can be incredibly frustrating to just get a boolean back as an error message when you don't know what is going on * Update error message to be consistent with the new style * Aligned error messages to be consistent with the new style * Fix ttsClearQueue to return nil+error, not actually error on invalid index * Minor code formatting * Floats to doubles * Update documentation links

vadi2 added 5 commits June 2, 2017 15:35

Improved name of the function getting all encoding names

9f44e6d

WIP

7777f23

Merge remote-tracking branch 'origin/development' into give-meaning-t…

0a49fe8

…o-encodings

Added all of the encodings back in

a97a7b6

Fixed preferences dialog to work again

daff373

vadi2 added the 1 - Ready label Jun 19, 2017

vadi2 requested review from SlySven and itsTheFae June 19, 2017 20:17

SlySven requested changes Jun 20, 2017

View reviewed changes

Fixed labels to be translatable

440d75d

vadi2 assigned itsTheFae Jun 20, 2017

itsTheFae approved these changes Jun 21, 2017

View reviewed changes

vadi2 added this to the 3.3 milestone Jun 21, 2017

vadi2 assigned SlySven and unassigned itsTheFae Jun 26, 2017

SlySven reviewed Jun 27, 2017

View reviewed changes

vadi2 assigned vadi2 and unassigned SlySven Jun 27, 2017

vadi2 added 2 commits June 27, 2017 14:46

Addressed feedback

3db292f

Merge remote-tracking branch 'origin/development' into give-meaning-t…

9ad6ec3

…o-encodings # Conflicts: # src/TBuffer.cpp # src/ctelnet.cpp # src/dlgProfilePreferences.cpp

vadi2 assigned SlySven and unassigned vadi2 Jun 28, 2017

SlySven approved these changes Jun 28, 2017

View reviewed changes

SlySven assigned vadi2 and unassigned SlySven Jun 28, 2017

vadi2 merged commit 5913a30 into Mudlet:development Jun 29, 2017

vadi2 deleted the give-meaning-to-encodings branch June 29, 2017 08:00

vadi2 added a commit that referenced this pull request Jul 2, 2017

Give meaning to encodings (#1107)

7653a29

Uh oh!

Conversation

vadi2 commented Jun 19, 2017

Uh oh!

SlySven left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SlySven Jun 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SlySven Jun 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vadi2 commented Jun 20, 2017

Uh oh!

itsTheFae left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vadi2 commented Jun 27, 2017

Uh oh!

SlySven left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SlySven Jun 27, 2017 •

edited

Loading

SlySven Jun 27, 2017 •

edited

Loading