D21287: Try to find always an unique visible document name without a number suffix

Dominik Haumann noreply at phabricator.kde.org
Sun May 19 14:52:57 BST 2019


dhaumann added a comment.


  Looking at the screenshots, I find it rather intransparent which document gets what name, besides that the quick open already shows the path to distinguish the files -> is this really an improvement?
  
  And: I have the feeling this patch should pay attention to performance. Sometimes, users have hundreds of files open, and this should not slow down any open operation.
  
  Since I am not yet convinced by this path: -1 right now...
  
  Any comments? :-)
  
  PS: The tab switcher also has something similar to make a distiction between files with the same name D16054 <https://phabricator.kde.org/D16054>

INLINE COMMENTS

> katedocument.cpp:4229
> +
> +    QString fullUrl = removeNewLines(url.url());
> +

missing const.

> katedocument.cpp:4235
> +
> +    QStringList splittedUrl = fullUrl.split(QLatin1Char('/'), QString::SkipEmptyParts);
> +

Could you use QString::splitRef() to avoid not needed QString allocations? You will get a QVector<QStringRef> instead of a QStringList.

> katedocument.cpp:4240
> +    // ...to avoid odd naming in rare cases we force numbering fallback
> +    if (splittedUrl.at(0) == QLatin1String("file:")) {
> +        splittedUrl.removeAt(0);

I wonder how this behaves on Windows :-)

> katedocument.cpp:4242
> +        splittedUrl.removeAt(0);
> +        if (splittedUrl.at(0) == QLatin1String("home")) {
> +            splittedUrl.removeAt(0);

Same here: Windows?

> katedocument.cpp:4273
> +    // Collect all files with same name...
> +    typedef QPair<KTextEditor::DocumentPrivate*, QStringList> FooPair;
> +    QList<FooPair> fooList;

FooPair? ...

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D21287

To: loh.tar, #ktexteditor
Cc: dhaumann, cullmann, kwrite-devel, kde-frameworks-devel, #ktexteditor, domson, michaelh, ngraham, bruns, demsking, sars
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kwrite-devel/attachments/20190519/dc14b073/attachment-0001.html>


More information about the KWrite-Devel mailing list