Try to find always an unique visible document name without a number suffix
Needs ReviewPublic

Authored by loh.tar on May 19 2019, 11:16 AM.

Details

Reviewers
None
Group Reviewers
KTextEditor
Summary

When you e.g. work on multiple projects at the same time you have often the case that you have documents with a number suffix on tabs or other places like side bars. These suffix give no hint which file it really is.
This patch add some folder name to the document to make the name unique, which should be in any case more helpful than the number suffix.

Drawbacks

  • The unique names can change when you open a file
  • The numbering is kept as fallback for rare cases. When it is in use are these numbers also not "static" like in case of "Untitled" documents
Test Plan

No Patch


Patch

A slightly more realistic example

Hint: The shown "Documents" plugin did not notice name changes, keep this in mind when you play around

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
loh.tar created this revision.May 19 2019, 11:16 AM
Restricted Application added projects: Kate, Frameworks. · View Herald TranscriptMay 19 2019, 11:16 AM
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel. · View Herald Transcript
loh.tar requested review of this revision.May 19 2019, 11:16 AM

I like the idea of having better short document names without numbers.
The algo looks a bit like what the document list tries to do with the shortest paths differentiating two files.
I think the documents stuff ignores the changed doc names as it looks for file name changes, but that might be wrong.

For the code: I would prefer that one uses e.g. just a call to an algorithm for reversing: https://en.cppreference.com/w/cpp/algorithm/reverse
For the data structures, please use some vector for the pairs, the QList will heap allocate all stuff larger than a pointer.

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

src/document/katedocument.cpp
4229

missing const.

4235

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

4240

I wonder how this behaves on Windows :-)

4242

Same here: Windows?

4273

FooPair? ...

PS: The tab switcher also has something similar to make a distiction between files with the same name D16054

I noticed this patch. It's more a workaround for the core issue.

besides that the quick open already shows the path to distinguish the files

Like D16054, but without harm. Not so bad to see the full path there

Looking at the screenshots, I find it rather intransparent which document gets what name,

Yes it is. Better solutions are welcome.
My examples, or better test files, are of course the worst thinkable case and not well suited to demonstrate the (hoped) benefit of this patch.

is this really an improvement?

It's the right place to fix the issue. You have then every where the unique names without extra quirks. Consider e.g. confirmation dialogues.

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.

Well, speedups are always nice. On session restore is it really often called and with my test setup the names often renamed. But should that really cause a noticable slowdown? I have doubts.

Perhaps has session restore potential for improvements, and the whole naming process too. The function is called for each file surprising often just to set the first true name. "Emtpty/NoName"->"Untitled"->"FileName"

loh.tar edited the test plan for this revision. (Show Details)May 19 2019, 3:45 PM

There even is a bug report about this: https://bugs.kde.org/show_bug.cgi?id=381532
In this bug report, there is also an example of how sublime text handles this, namely "Document name - path/bla", i.e. the path is appended behind the document name. What about this solution?

Algo:

  1. When opening a file, identify all other open documents with the same filename
  2. For all these documents, create the unique names by appending the path that uniquely identifies the files

I think for 2 one still needs a clever way to compute the "short" path. I think the document list tries to create shortest unique "suffixes".
For 1) to be efficient one could hash all documents with "filename" => documents with that if file names changes, that would avoid that one needs to iterate always again over all documents to collect duplicates.

the path is appended behind the document name. What about this solution?

Don't like that idea. And fit not well to the shown "Document List" problem.

to be efficient one could hash all documents

I'm always in favor for such. But here I had no idea how to do that nicely and reliable. One Document is not the place to hold such hash, and the patch lives only there.

The "file rename case" may indeed not catch here when it goes from duplicate to unique, but how often is that the case? Guess nearby never and it is no big harm to keep the prefix in that case. "Untitled" documnets are handled as before.

that would avoid that one needs to iterate always again over all documents to collect duplicates.

Is a loop running once over 10 to 1000 (one should rethink his workflow when he has so many files open) turns on a file name update such an issue and any optimization worth? Well, currently lead that on session restore to one million loops, how long may take that? And compared to the other needed work to open one file. Guess it's not a big thing.