Differential D17956

DocumentPrivate: Fix broken doc links in qCWarning
ClosedPublic
Actions

Authored by loh.tar on Jan 4 2019, 5:07 AM.

Details

Reviewers
sars
dhaumann
ltoscano
Group Reviewers
KTextEditor

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
loh.tar created this revision.Jan 4 2019, 5:07 AM
Restricted Application added projects: Kate, Frameworks. · View Herald TranscriptJan 4 2019, 5:07 AM
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel. · View Herald Transcript
loh.tar requested review of this revision.Jan 4 2019, 5:07 AM
loh.tar added a comment.Jan 4 2019, 5:11 AM

How about to change this logging into a warning message in the view? Or will it displayed(?)
I have noticed these while investigating https://bugs.kde.org/show_bug.cgi?id=397956

sars accepted this revision.Jan 4 2019, 7:10 AM
This revision is now accepted and ready to land.Jan 4 2019, 7:10 AM
sars added a comment.Jan 4 2019, 7:14 AM

Adding as a warning message in the view could be another review and needs comments from others first

ltoscano requested changes to this revision.Jan 4 2019, 9:20 AM
ltoscano added a subscriber: ltoscano.
ltoscano added inline comments.
src/document/katedocument.cpp
4592 ↗(On Diff #48655)

Please use:

https://docs.kde.org/index.php?application=katepart&branch=stable5&path=config-variables.html#variable-remove-trailing-spaces

4597 ↗(On Diff #48655)

See above

This revision now requires changes to proceed.Jan 4 2019, 9:20 AM
pino added a subscriber: pino.Jan 4 2019, 11:15 AM

Also, I do not understand why a console warning is translated: normal users will not see it, and if they do the message is not actionable for them.

cullmann added a reviewer: dhaumann.Jan 4 2019, 1:43 PM
cullmann added a subscriber: cullmann.

Should we perhaps just remove that message and keep the old compat code without any hint here?
I think it doesn't hurt to just have the few extra checks "forever".

loh.tar updated this revision to Diff 48671.Jan 4 2019, 1:45 PM

Requested modified link

ltoscano resigned from this revision.Jan 4 2019, 1:48 PM

Fine by me regarding the change; I can't answer to the question whether the warning should be localized or not (maybe not), so I'm resigning from the review to not block it

This revision is now accepted and ready to land.Jan 4 2019, 1:48 PM
cullmann closed this revision.Jan 20 2019, 12:12 PM

Fixed links got commited in meantime:

commit b64681408d840880c7625543992cc6a75535944a
Author: Grzegorz Szymaszek <gszymaszek@short.pl>
Date: Thu Jan 10 08:42:46 2019 +0100

Use HTTPS, if available, in links visible to users

Reviewers: #ktexteditor, dhaumann

Reviewed By: #ktexteditor, dhaumann

Subscribers: lueck, kwrite-devel, kde-frameworks-devel

Tags: #kate, #frameworks

Differential Revision: https://phabricator.kde.org/D18148

Revision Contents
Changeset List

PathPackages
Msrc/document/katedocument.cpp (4 lines)

Diff 48671

View Options

src/document/katedocument.cpp

Loading...