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

Repair colors of translated texts #2005

Closed
Kebap opened this issue Sep 27, 2018 · 7 comments · Fixed by #2018
Closed

Repair colors of translated texts #2005

Kebap opened this issue Sep 27, 2018 · 7 comments · Fixed by #2018

Comments

@Kebap
Copy link
Contributor

Kebap commented Sep 27, 2018

Brief summary of issue / Description of requested feature:

We now need to fix the bug where the [ INFO ] [ WARNING ] etc code colours text based on the presence of those words. The translated strings don't get coloured correctly :-P

Steps to reproduce the issue / Reasons for adding feature:

  1. Switch to different displayed language
  2. Observe warnings and informational texts. For example upon connecting profiles.
  3. Compare colors to English original

Error output / Expected result of feature

Display should be the same

Extra information, such as Mudlet version, operating system and ideas for how to solve / implement:

This should not work over comparing strings at all.. How about refactoring this and creating some functions like give_info(text) and give_warning, etc. Then only they will take care of adding the desired color and prefix titles there. They also handle wrapping and indentation of following lines. Then all other instances can just call them with arbitrary text to display and without prefix, without 2 space characters(!), without predefined wrapping, etc. The translation teams need not bother neither. If we want to rename any prefix, it will be possible in one central place instead of needing dozens of adjustments all over the places.

@Kebap
Copy link
Contributor Author

Kebap commented Sep 27, 2018

@SlySven Thanks for elaborating the structure of existing prefixes in the issue linked above.

Another question arose during translation: How do you distinguish between "ALERT" and "WARN" ? They both translate very alike for me.

@SlySven
Copy link
Member

SlySven commented Sep 27, 2018

Presay? Do you mean Prefix?

If you are going to redesign the cTelnet::postMessage(const QString&) method you will want to note the following about the current implementation:

  • During start-up some messages arrive before they can be displayed - so they get stacked up and then the first message that comes along when they CAN be shown triggers all of them to be sent to the screen in order of the calls to postMessage that made them - and if the class is destroyed before they can be shown they still get sent to the standard output...
  • The first line of the message is left aligned but any additional lines in the QString supplied to the method are indented to the same number of space as the one after the hyphen (well actually the minus sign -) by the code that slices the first line of the message into the prefix and remainder. If the different message types are going to be handled by an overload or a number of methods with different names but the same signature then I guess the existing (QStringList) cTelnet::messageStack could be changed to a QList <QPair<quint8, QString>> and the first member of the QPair can be the message type.
  • Tabs were an issue - a workaroundbodge was used to replace them with spaces but IIRC an improvement to the text painting code (possibly TTextEdit::drawForeground() or near to that) may have improved the situation...
  • in response to a recent comment I am of the opinion that an ALERT is something that the user needs to know about but which is not necessary bad whereas a WARN has negative connotation but is not so bad that it is an ERROR. E.g.:
    • ALERT - There is a strange dog outside my house 🐶
    • WARN - My cat is at the Front door and the dog has just seen it 🙀
    • ERROR - I had not remember to unlock the cat-flap and my cat was unable to escape... 🕇

@Kebap
Copy link
Contributor Author

Kebap commented Sep 30, 2018

Yes, prefix. No, not Prefix.. ;)

OK then my translation of ALERT was utterly wrong. I took it as ALARM (dog is attacking the cat) and therefore even worse than a mere WARNING

@Kebap Kebap added Hacktoberfest Code & win a T-shirt :) and removed Hacktoberfest Code & win a T-shirt :) labels Oct 1, 2018
@vadi2 vadi2 self-assigned this Oct 3, 2018
@vadi2
Copy link
Member

vadi2 commented Oct 3, 2018

I think an ALERT should be just an INFO because if we're getting it wrong between ourselves, surely our users are as well.

@Kebap
Copy link
Contributor Author

Kebap commented Oct 3, 2018

You closed this issue, but there could be two follow up issues: Change ALERT to INFO, and refactor the message output as described above. Should I open 1-2 new issues, or re-open this one? Will re-open this for now, not to lose the information.

@Kebap Kebap reopened this Oct 3, 2018
@vadi2
Copy link
Member

vadi2 commented Oct 3, 2018

Github auto-closes the issue - but yes, the refactoring is a feature, the actual problem is fixed.

@Kebap Kebap removed the Hacktoberfest Code & win a T-shirt :) label Nov 5, 2018
@Kebap
Copy link
Contributor Author

Kebap commented Feb 5, 2019

Closing this original issue as fixed after creating new issues for new issues.

@Kebap Kebap closed this as completed Feb 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants