Added TDE license info dialog #318
Merged
MicheleC
merged 1 commits from feat/tde-license-info
into master
1 year ago
Loading…
Reference in new issue
There is no content yet.
Delete Branch 'feat/tde-license-info'
Deleting a branch is permanent. It CANNOT be undone. Continue?
As per title. The dialog is shown once per minor TDE release.
On deb distros, to be built with TDE/tde-packaging#212
8434760d67
to7321c59117
1 year agoI haven't tested this but I took a quick glance at the code. Here's some notes:
$TDEPREFIX/share/apps/LICENSES/
. Approximate code:I've taken a look through
tdebase
and created this 'little' table of the licenses for each component:https://wiki.trinitydesktop.org/User:Blu256/Draft:Licenses
The licenses were determined by:
a) Looking at TDEAboutData
b) Looking into COPYING and LICENSE* files
c) Looking at the source code files' header comments
I don't know much about licensing, but I've found that apart from the four licenses you have provided in TDELicenseDlg, there's also the MIT license, the 2-clause BSD license and the Artistic license (and also GNU LGPL version 2.1). Some of these only seem to apply to specific source files, while others for whole components (e.g. the Artistic license seems to apply to TDESU, KDCOP, tdepasswd and kstart).
What do you think about this?
Yes, definitely, licenses texts are already part of the installation, so it makes sense to use these existing files instead of the embedding of texts inside the binary.
Hi Philippe,
thanks for the good feedback.
Indeed it is a better idea, I didn't realize we were already distributing the files with TDE (although I should have, I guess!).
I didn't know/remember there is such class. I will test and see how it looks and if it looks good, we can use that.
As per point 1., didn't realize TDE is using more licenses. We definitely need to add the missing ones, so I will amend the code as needed.
Sounds like a good idea. Maybe even better, we could add a tab "Licenses" that contains the same contents of the tde_licence_info dialog.
What do you think?
7321c59117
todf28191774
1 year agoPR updated based on the feedback from Philippe. I have added the remaining 3 licenses that were part of TDEAboutData but were not included in the original PR. The license text is read from file (thanks for the code :-) ) and I replaced the KTextEdit (which was supposed to be readonly but I had forgotten one line of code) with a KTextBrowser.
Point 4. has not been implemented yet.
Just discussed with Slavek that it will be better to add the MIT license too as pointed out by Philippe, so more changes to come (here and in tdelibs).
df28191774
toae275b8f7b
1 year agoMIT license also added. This requires TDE/tdelibs#197.
The dialog looks like this:
Both approarches sound good.
Good work! I might test it later today.
Probably should be added to TDEAboutData class too (just saying...).
done, see TDE/tdelibs#197
Looks a little unusual to me from a UI point of view. Maybe we could use tabs and preload the licenses into them? This would look more "usual" and would avoid reloading each license when you select another license (thus supposedly making the UI snappier when it has loaded).
For point 4. it seems to be a good idea to add a License tab, because in this way it is also in About dialog for applications. The question is whether to insert as much content there as it is in the newly added separate window, as this could cause a significant window enlargement.
After further tihnking, adding a tab is probably the better of the two. This will require moving most of the code from tdebase into tdelibs (like ktips does) and then loading the contents of the dialog in the related container widget (either the new tab of the tde_license_info dialog).
Since R14.1.0 is going to be soft frozen tonight, I suggest we leave point 4. for R14.1.1. @blu.256 @SlavekB what do you think?
Seems reasonable. You can merge the dialog in its current form for R14.1.0 and then come back to improve it in R14.1.1.
I thought about doing pre-loading, but in the end you would load all files regardless of whether the user clicks on the buttons. Most likely, users won't read those licenses IMO, so lazy loading on demand seems more suitable.
I agree, but my main point was using a KTabWidget instead of buttons. Pre-loading was just a side-effect that would stem from it.
Using KTabWidget also seems to me like a good idea.
ae275b8f7b
to0308f2af79
1 year agoUpdated the code to use KTabWidget. It now looks like this:
Strangely, if I use
&
in the tab label, it gets shown as such rather than being used as shortcut for that tab :-(0308f2af79
to6c423143f8
1 year agoUpdated dialog box with KTabWidget seems good to me. BTW, we should add CMakeL10n rules to generate a new
TDELicense.pot
catalog.6c423143f8
to9b5a8c93ce
1 year agoPR updated
Looks good!
It is necessary to decide on the name of the instance × the name of the translation catalog.
tde_l10n_create_template(
CATALOG "tdelicense"
SOURCES "*.cpp"
Because
*.cpp
files are part of the defaults, unless otherwise specified, then there is no need to explicitly determineSOURCES
. In this case, it is also possible to omitCATALOG
. So here it can be simplified to:However, the catalog name must be the same as the instance name of the application. And because "TDELicense" is used for
TDEAboutData
inmainWindow.cpp
, I assume that the same lower/UPPER case is needed.Do you propose to change the name of the catalog or instance name of the application? Or in the application to add explicit determination of the name of the translation catalog?
int main(int argc, char *argv[])
{
TDEAboutData aboutData("TDELicense", I18N_NOOP("TDE License"),
It is necessary to unify the name of the instance and the name of the catalog with translations or to use:
Good, thanks for the feedback
9b5a8c93ce
to3485003684
1 year ago3485003684
tob957aab3e4
1 year agoAll looks good.
b957aab3e4
into master 1 year agoReviewers
b957aab3e4
.