Use template text when naming the generated clip
Needs ReviewPublic

Authored by Zren on Apr 9 2018, 11:40 PM.

Details

Reviewers
None
Group Reviewers
Kdenlive
Summary

Prefix with Title: so that sorting the clips by name keeps all titles together. We could shorten it to [T] or T: . We could remove the prefix altogether too, but we'd lose the sorting advantage.
32 characters should be long enough, though I'm not certain how right to left languages will grab the start or the end.

Diff Detail

Repository
R158 Kdenlive
Lint
Lint Skipped
Unit
Unit Tests Skipped
Zren requested review of this revision.Apr 9 2018, 11:40 PM
Zren created this revision.
Zren edited the summary of this revision. (Show Details)Apr 9 2018, 11:44 PM
frdbr added a subscriber: frdbr.Apr 10 2018, 1:24 AM

@Zren happy to see you participating in the project. Welcome aboard! If you are interested in working more in the Titler here are some suggestions:

https://phabricator.kde.org/T3020

Cheers :D

Thanks for this. I would say 32 characters is a bit long. Maybe 20 ? Also you should use .simplified() instead of trimmed():

https://doc.qt.io/qt-5/qstring.html#simplified

It removes whitespaces and line returns.
And one last general thing regarding Kdenlive's current situation: we are currently in the middle of a huge refactoring. This refactoring is living in the "refactoring_timeline" branch. There was a LOT of code changes.

This refactoring branch will be merged to git master in about one month. So currently, I would not advise to work on the git master branch, but rather on the refactoring_timeline so it is easier for us to merge. However the title stuff is one of the few parts that has almost not changed between master and refactoring. So if it's easier for you and you only work on small improvements that touch just 1-2 files related to titles, it's ok to work in master and I can manage the merge with refactoring.

And welcome to Kdenlive :)

Ah just forgot, in the refactoring branch, this feature (using first lines of text for title clip name) is already implemented for normal title clips. So your change will fit nicely.

Zren added a comment.Apr 10 2018, 8:17 AM
rather on the refactoring_timeline

I'm actually building on the Applications/17.12 branch atm since KDE Neon couldn't build the master branch as it has an older versioned dependency.

Could NOT find MLT: Found unsuitable version "6.4.1", but required is at least "6.6.0" (found /usr/lib/x86_64-linux-gnu/libmlt.so)

I was checking to make sure the code hadn't change in master though. I guess I'll check refactoring_timeline instead.

So if you approve a commit, merge to refactoring_timeline instead? Can do.

simplified()

TIL about that function. It's not usually in a language's standard library so never thought to look for it.

already implemented for normal title clips

Oh awesome. Was a bit intimidated at trying to parse out all the XML text elements.

Hmmm, looks like I'll need to figure out how to compile that branch as it's been changed.

It also looks like the normal title clips are named with:

title = currentTitle.length() > 12 ? currentTitle.left(12) + QStringLiteral("...") : currentTitle;

So I'll toss a simplified() for those too.

Oh right, Kdenlive refactoring requires MLT >= 6.6.0. You can find instructions on building MLT here:
https://community.kde.org/Kdenlive/Development/KF5#Compiling_MLT_with_Qt5_support

Since Neon is Ubuntu based you can follow the described steps. But anyways, yes if you want to provide patches against master I can do the merges. And yes, the normal title misses the simplified() call!

Let me know if you have any question.