Create Task
Maniphest T13913

Port Frameworks away from QStringRef
Open, NormalPublic
Actions

Description

According to [1], QStringRef will be in a qt5compat module.

[1]https://code.qt.io/cgit/qt/qt5compat.git/tree/src/core5/text

The plans have changed, and now we actually require Qt5.15, so we should start porting away from QStringRef; some stuff will have to wait until the branching, e.g. QStringView::toInt() in Qt5 is implemented just to keep code working but isn't efficient (see the QStringView docs for such issues, things like toInt() should wait till the branching or we could use #ifdef kludges... again).

Frameworks that have QStringRef|rightRef|midRef|leftRef:

  • attica
  • baloo
  • kactivities-stats
  • kbookmarks
  • kcalendarcore
  • kcompletion
  • kconfig
  • kcontacts
  • kcoreaddons
  • kfilemetadata
  • kguiaddons
  • ki18n
  • kiconthemes
  • kirigami
  • knewstuff
  • knotifications (only in some windows-sepcific code)
  • kpackage
  • kservice
  • ktexteditor
  • ktextwidgets
  • kwayland
  • kwidgetsaddons
  • plasma-framework
  • solid
  • sonnet https://invent.kde.org/frameworks/sonnet/-/merge_requests/20
  • syntax-highlighting
ahmadsamir created this task.Nov 29 2020, 9:47 AM
ahmadsamir triaged this task as Normal priority.
ahmadsamir added a comment.Nov 29 2020, 10:03 AM

IIUC, the candidate replacement is QStringView.

nicolasfella added a subscriber: nicolasfella.Nov 29 2020, 2:26 PM

correct. Unfortunately a part of the relevant QStringView is only available in Qt 5.15.2 so it will be a while before we can use that in Frameworks without ifdefs

This is also a good opportunity to explore where using QStringView in the API makes sense

ahmadsamir added a comment.Nov 29 2020, 3:12 PM

Checking whether a QStrinView is available in 5.13 may be a pain in the neck... (maybe a cheat-sheet would be helpful here... I may actually look into making one :)).

ahmadsamir added a comment.Nov 30 2020, 10:31 AM

The #if kludges will sure look ugly...

nicolasfella added a comment.Nov 30 2020, 12:21 PM

The API introduced in Qt 5.15.2 is only meant as a porting aid since the implementation is ugly/slow.

See https://codereview.qt-project.org/c/qt/qtbase/+/324103

dfaure added a subscriber: dfaure.Nov 30 2020, 12:55 PM

"a qt5combat module" << excellent typo ! :-)

ahmadsamir added a comment.Nov 30 2020, 12:57 PM
In T13913#245730, @nicolasfella wrote:

The API introduced in Qt 5.15.2 is only meant as a porting aid since the implementation is ugly/slow.

See https://codereview.qt-project.org/c/qt/qtbase/+/324103

I saw notes about that in the API docs. Which means we shouldn't use it in current KF5 code, which will be used by our users for some time until KF6 is ready for release... loopy problems.

ahmadsamir added a comment.Nov 30 2020, 12:58 PM
In T13913#245732, @dfaure wrote:

"a qt5combat module" << excellent typo ! :-)

hmmm, given how much ifdef'ery we may end up with...

ahmadsamir added a comment.Nov 30 2020, 1:01 PM

(Fixed typo in wiki page, and added a note about the ugly/slow porting aids and a link to this task).

nicolasfella added a comment.Feb 28 2021, 8:28 PM

Fortunately it is not used much in public frameworks API. The only usage I found is in ktexteditor and it's already marked for removal

nicolasfella added a comment.Mar 1 2021, 1:14 PM

One thing that QStringRef offers that QStringView seems to miss is position().

Sonnet makes heavy use of that

ahmadsamir updated the task description. (Show Details)Apr 26 2021, 12:49 PM
ahmadsamir moved this task from Waiting on KF6 Branching to Backlog on the KF6 board.Apr 29 2021, 2:39 PM
ahmadsamir updated the task description. (Show Details)
alex updated the task description. (Show Details)Apr 29 2021, 3:37 PM
ahmadsamir updated the task description. (Show Details)Apr 29 2021, 8:02 PM
ahmadsamir updated the task description. (Show Details)Apr 30 2021, 7:27 PM
ahmadsamir updated the task description. (Show Details)May 2 2021, 9:38 AM
ahmadsamir updated the task description. (Show Details)May 2 2021, 6:03 PM
ahmadsamir added a comment.May 2 2021, 11:44 PM

Looks like this will definitely need to wait till the branching https://invent.kde.org/pim/kpimtextedit/-/merge_requests/11#note_233246

ahmadsamir updated the task description. (Show Details)Oct 13 2021, 5:23 PM

Updated based on a recent grep on the Frameworks. Either they're ported or an MR has been created.

ahmadsamir updated the task description. (Show Details)Oct 13 2021, 5:25 PM
vkrause moved this task from Waiting on KF6 Branching to Done on the KF6 board.Jan 29 2023, 2:31 PM