D14360: Remove custom icon selection for trash

2018-07-29 Thread Shubham
shubham added a comment.


  ping?

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, broulik, #dolphin, #frameworks, pino
Cc: pino, kde-frameworks-devel, michaelh, ngraham, bruns


D14449: Display used space in GiB also

2018-07-29 Thread Dominik Haumann
dhaumann added a comment.


  We have other places where such information is displayed, e.g. the disk quota 
applet: 
https://kate-editor.org/2015/08/02/plasma-5-keeping-an-eye-on-the-disk-quota/
  Maybe this is useful for input :)

REPOSITORY
  R241 KIO

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

To: shubham, elvisangelaccio, ngraham, #frameworks
Cc: rkflx, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


D14449: Display used space in GiB also

2018-07-29 Thread Henrik Fehlauer
rkflx added a comment.


  In D14449#300327 , @ngraham wrote:
  
  > Here's another idea for how to express everything while remaining compact 
in the horizontal dimension:
  >
  >   Device capacity: -- 24% full
  >22.5 GiB used, 71.9 GiB free
  >94.4 Gib total size
  
  
  Not bad at all, but due to using 3 lines now puts quite some emphasis on the 
device, while the dialog is about the folder actually. Would this be a 
compromise?:
  
Device capacity: ==- 24% used of 94.4 Gib
 22.5 GiB used, 71.9 GiB free
  
  (Not too happy about the duplication of "used", though.)

REPOSITORY
  R241 KIO

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

To: shubham, elvisangelaccio, ngraham, #frameworks
Cc: rkflx, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


Re: Pre-review CI

2018-07-29 Thread Bhushan Shah
On Sun, Jul 29, 2018 at 05:00:19PM +0200, Friedrich W. H. Kossebau wrote:
> 1 job means one huge build log to look at, or? In that case I would prefer 
> separate jobs. Given review requests are prone to fail.

Thing is, you don't care about the pre-review CI jobs as long as they
are passing, but in case of fail, yes you might have to look at long
log depending on where failure was, in first platform or last one.

> compare to non-review build jobs, I would assume. And having jobs separate 
> also means one gets results for any platforms, does not stop on the first 
> failing?

Yes, but it also means that if there is obvious mistake in review, all
will fail, insteaad of bailing out earlier.

> All that said though without having seen how a one-job-to-rule-them-all would 
> look like, and how the others. Any chance for some samples, please?

We haven't enabled multi-platform jobs currently but it would be
basically for each platform (linux, windows, FreeBSD, Android). It will
execute them in order, and if one fails, it will just bail out.

> > - Should we send out comment for failure and success? Or is it easier to
> >   figure out the console log link without the comment? See linked review
> >   for example [1].
> 
> [1] -> [2] here.
> 
> What do you mean exactly by "send out comment for failure and success"? More 
> emails? (Please not). That example works fine with me, but not sure what the 
> alternative is?

The alternative being, instead of jenkins-ci comemnting with link, you
find it manually in Diff detail section, see screenshot

https://screenshots.firefox.com/1GG7B0bPLod3QMFg/phabricator.kde.org

Here, the "Jenkins" after the "Build 1313: Frameworks Pre-Review CI" is
link to jenkins build. But the test was if you were able to spot it in
first glance, and it failed. :P so yeah we will keep comments enabled
even if it means extra emails.

> Cheers and Thanks again for improving the review system
> Friedrich

Thanks

-- 
Bhushan Shah
http://blog.bshah.in
IRC Nick : bshah on Freenode
GPG key fingerprint : 0AAC 775B B643 7A8D 9AF7 A3AC FE07 8411 7FBC E11D


signature.asc
Description: PGP signature


D13869: [solid] Notify when interface to mounted fs is lost

2018-07-29 Thread Anthony Fieroni
anthonyfieroni added a comment.


  Ping, anything unclear ?

REPOSITORY
  R245 Solid

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

To: anthonyfieroni, broulik, cfeck, dfaure
Cc: bcooksley, bruns, kde-frameworks-devel, michaelh, ngraham


KDE CI: Frameworks kio kf5-qt5 FreeBSDQt5.10 - Build # 94 - Still Unstable!

2018-07-29 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20FreeBSDQt5.10/94/
 Project:
Frameworks kio kf5-qt5 FreeBSDQt5.10
 Date of build:
Sun, 29 Jul 2018 19:16:31 +
 Build duration:
6 hr 57 min and counting
   JUnit Tests
  Name: (root) Failed: 5 test(s), Passed: 53 test(s), Skipped: 0 test(s), Total: 58 test(s)Failed: TestSuite.kiocore-jobtestFailed: TestSuite.kiocore-kmountpointtestFailed: TestSuite.kiofilewidgets-kfileplacesviewtestFailed: TestSuite.kiowidgets-kdirlistertestFailed: TestSuite.kiowidgets-kdirmodeltest

D14449: Display used space in GiB also

2018-07-29 Thread Nathaniel Graham
ngraham added a comment.


  +1 for using the same label but putting the information on more than one line 
like @rkflx suggests.
  
  I think there are basically three important pieces of information here:
  
  - Percentage free (for people who don't understand what a "GiB" is--the 
"Simple by default" folks)
  - Amount of free space left (for people who know what a GiB is and want hard 
numbers)
  - Total disk capacity (for the above group of people)
  
  The amount of space used is non-critical, which is probably why it wasn't 
displayed before, but I don't see the harm in adding it if we can avoid using 
too much horizontal space. Here's another idea for how to express everything 
while remaining compact in the horizontal dimension:
  
Device capacity: -- 24% full
 22.5 GiB used, 71.9 GiB free
 94.4 Gib total size

REPOSITORY
  R241 KIO

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

To: shubham, elvisangelaccio, ngraham, #frameworks
Cc: rkflx, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


D10040: Add serial number and EISA ID to OutputDevice interface

2018-07-29 Thread David Edmundson
davidedmundson updated this revision to Diff 38734.
davidedmundson added a comment.


  exciting whitespace change

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10040?vs=38733=38734

BRANCH
  output_changes

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

AFFECTED FILES
  autotests/client/test_wayland_outputdevice.cpp
  src/client/outputdevice.cpp
  src/client/outputdevice.h
  src/client/protocols/outputdevice.xml
  src/server/outputdevice_interface.cpp
  src/server/outputdevice_interface.h

To: davidedmundson, graesslin, sebas, #kwin, dvratil
Cc: kde-frameworks-devel, davidedmundson, plasma-devel, ragreen, Pitel, 
schernikov, michaelh, ZrenBot, ngraham, bruns, alexeymin, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein


D10040: Add serial number and EISA ID to OutputDevice interface

2018-07-29 Thread David Edmundson
davidedmundson updated this revision to Diff 38733.
davidedmundson added a comment.


  Instead of modifying the geometry event use two new events to remain
  fully compatiable.

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10040?vs=25806=38733

BRANCH
  output_changes

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

AFFECTED FILES
  autotests/client/test_wayland_outputdevice.cpp
  src/client/outputdevice.cpp
  src/client/outputdevice.h
  src/client/protocols/outputdevice.xml
  src/server/outputdevice_interface.cpp
  src/server/outputdevice_interface.h

To: davidedmundson, graesslin, sebas, #kwin, dvratil
Cc: kde-frameworks-devel, davidedmundson, plasma-devel, ragreen, Pitel, 
schernikov, michaelh, ZrenBot, ngraham, bruns, alexeymin, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein


D14457: Forward-declare X509 structure

2018-07-29 Thread Luca Carlon
luc4 retitled this revision from "Forward-define X509 structure" to 
"Forward-declare X509 structure".
luc4 edited the summary of this revision.

REPOSITORY
  R239 KDELibs4Support

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

To: luc4
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


Re: gettext makes supporting multiple languages translations in ki18n impossible

2018-07-29 Thread Albert Astals Cid
El diumenge, 29 de juliol de 2018, a les 22:26:56 CEST, Albert Astals Cid va 
escriure:
> El diumenge, 29 de juliol de 2018, a les 21:45:27 CEST, Karl Ove Hufthammer 
> va escriure:
> > Albert Astals Cid skreiv 29. juli 2018 19:31:
> > > Ideally one would be able to query gettext about whether the translation 
> > > exists or not (basically having an option to return nullptr in the above 
> > > function or similar), but I have not been able to find any such 
> > > functionality in glibc/libintl itself or in any other library out there.
> > 
> > According to comment #7 at https://bugs.kde.org/show_bug.cgi?id=168134, 
> > this is possible:
> > 
> > 
> > > […] how to know which language did the translation came from. Turned out 
> > > that raw gettext calls will return the same const char* pointer given to 
> > > them if there was no translation, as opposed to different pointer if the 
> > > translation was there, but same string-wise. Since i18n() uses these raw 
> > > calls under the hood, by this it can check the language.
> 
> 
> Right, that makes sense, i'll investigate.

https://phabricator.kde.org/D14473

> 
> Cheers,
>   Albert
> 
> 
> 
> 






D14473: Fix KCatalog::translate when translation is same as original text

2018-07-29 Thread Albert Astals Cid
aacid created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
aacid requested review of this revision.

REVISION SUMMARY
  We need to be compare the pointer passed to gettext with the one we get
  back.
  
  If it is the same it means no translation was found, if it
  is different then it means the translation was found.

TEST PLAN
  Test now succeeds, without patch it fails

REPOSITORY
  R249 KI18n

BRANCH
  master

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

AFFECTED FILES
  autotests/klocalizedstringtest.cpp
  autotests/klocalizedstringtest.h
  autotests/po/ca/ki18n-test.po
  src/kcatalog.cpp

To: aacid
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D14472: Set Dialog flag for JobDialog

2018-07-29 Thread Nicolas Fella
nicolasfella edited the test plan for this revision.

REPOSITORY
  R495 Purpose Library

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

To: nicolasfella, apol
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D14472: Set Dialog flag for JobDialog

2018-07-29 Thread Nicolas Fella
nicolasfella created this revision.
nicolasfella added a reviewer: apol.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
nicolasfella requested review of this revision.

REVISION SUMMARY
  The JobDialog window is treated as a "normal" window. When using a tiling 
window manager it gets as much space assigned as the other windows, which is 
quite unnecessary. With this patch it has the same size as when using a 
  floating WM and is floating above the "normal" windows (e.g. Dolphin), which 
seems more appropriate.

REPOSITORY
  R495 Purpose Library

BRANCH
  dialog

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

AFFECTED FILES
  src/widgets/JobDialog.qml

To: nicolasfella, apol
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


Re: gettext makes supporting multiple languages translations in ki18n impossible

2018-07-29 Thread Albert Astals Cid
El diumenge, 29 de juliol de 2018, a les 21:45:27 CEST, Karl Ove Hufthammer va 
escriure:
> Albert Astals Cid skreiv 29. juli 2018 19:31:
> > Ideally one would be able to query gettext about whether the translation 
> > exists or not (basically having an option to return nullptr in the above 
> > function or similar), but I have not been able to find any such 
> > functionality in glibc/libintl itself or in any other library out there.
> 
> According to comment #7 at https://bugs.kde.org/show_bug.cgi?id=168134, 
> this is possible:
> 
> 
> > […] how to know which language did the translation came from. Turned out 
> > that raw gettext calls will return the same const char* pointer given to 
> > them if there was no translation, as opposed to different pointer if the 
> > translation was there, but same string-wise. Since i18n() uses these raw 
> > calls under the hood, by this it can check the language.


Right, that makes sense, i'll investigate.

Cheers,
  Albert






D14471: Add Theme::TextStyle Format::textStyle() const;

2018-07-29 Thread Dominik Haumann
dhaumann created this revision.
dhaumann added reviewers: cullmann, vkrause.
Restricted Application added projects: Kate, Frameworks.
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel.
dhaumann requested review of this revision.

REVISION SUMMARY
  This is required for using KSyntaxHighlighting in KTextEditor.

TEST PLAN
  make && make test

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  extendFormat (branched from master)

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

AFFECTED FILES
  autotests/theme_test.cpp
  src/lib/format.cpp
  src/lib/format.h

To: dhaumann, cullmann, vkrause
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann


Re: how to get the list of keywords in KSyntaxHighlighting

2018-07-29 Thread Dominik Haumann
On Sun, Jul 29, 2018 at 6:16 PM, Alexander Semke  wrote:
> Hi Dominik,
>
>> sorry for the delay, I only now saw your mail.
>> Jonathan meanwhile posted a patch that adds this:
>> https://phabricator.kde.org/D14434
> Yes, I saw it already. Will this make into the 5.49 release of the frameworks?
>
>
>> However, pushing this further, what Kate also needs is a way go get
>> all keyword lists that are matched in the current context. This allows
>> for instance for context based auto completion that Kate also supports.
>>
>> Would that be of interest for you as well
> I think this makes sense. At the moment there is some custom logic for
> completion in Cantor's code. To refactor all this logic will take more time.
>
> Now I'd simply remove the manual maintanence of keywords in Cantor like it is
> done for Maxima at the moment (keywords orginally copied from Kate's code).
> For some other "backend systems" like Octave and R the keywords are obtained
> dynamically during the runtime from the backend which makes the presence of
> the actual backend system required, even if the user simply wants to _read_ a
> saved project. This is what I want to re-factor now to make https://
> phabricator.kde.org/T4760 possible - the user should be able to read Cantor's
> worksheets without the backend system being present on the target system.
> Moving to KSyntaxHighlighing with the new functions added by Jonathan will
> make this possible.
>
> So, if it's not a big problem to extend the APIs as done by Jonathan now,
> let's go for it even though Cantor is maybe the only consumer of this APIs
> now.

Thanks for this explanation. If you have a use case then I agree with
this change. Also getting the keyword lists of a Definition allows for
better unit testing, so I think this is also a minor reason for
approving this patch.

Thanks,
Dominik


D14434: add functions to access keywords

2018-07-29 Thread Dominik Haumann
dhaumann added a comment.


  @asemke Are you sure you reference the correct Task? I cannot find anything 
about keyword lists in your link.
  
  Besides that, my change request with respect to API naming still stands.
  
  @jpoelen Before committing, lets have another review round, could you please 
provide an updated patch?

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  keywordlist

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

To: jpoelen, #framework_syntax_highlighting, dhaumann, asemke
Cc: vkrause, cullmann, asemke, kde-frameworks-devel, kwrite-devel, michaelh, 
kevinapavew, ngraham, bruns, demsking, sars, dhaumann


KDE CI: Frameworks kio kf5-qt5 SUSEQt5.9 - Build # 184 - Still Unstable!

2018-07-29 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.9/184/
 Project:
Frameworks kio kf5-qt5 SUSEQt5.9
 Date of build:
Sun, 29 Jul 2018 19:16:31 +
 Build duration:
6 min 42 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 58 test(s), Skipped: 0 test(s), Total: 59 test(s)Failed: TestSuite.kiofilewidgets-kfileplacesviewtest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)65%
(258/396)65%
(258/396)53%
(31906/59887)38%
(16116/42650)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(55/55)100%
(55/55)95%
(9006/9436)51%
(3924/7700)autotests.http100%
(5/5)100%
(5/5)99%
(581/582)68%
(113/166)autotests.kcookiejar100%
(1/1)100%
(1/1)91%
(179/197)72%
(49/68)src100%
(1/1)100%
(1/1)86%
(6/7)67%
(4/6)src.core84%
(98/116)84%
(98/116)58%
(8331/14357)50%
(4653/9289)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets76%
(28/37)76%
(28/37)49%
(3895/7920)34%
(1587/4665)src.gui100%
(2/2)100%
(2/2)94%
(103/109)74%
(49/66)src.ioslaves.file100%
(5/5)100%
(5/5)52%
(527/1015)39%
(315/814)src.ioslaves.file.kauth0%
(0/2)0%
(0/2)0%
(0/106)0%
(0/65)src.ioslaves.ftp0%
(0/1)0%
(0/1)0%
(0/1364)0%
(0/1414)src.ioslaves.help0%
(0/5)0%
(0/5)0%
(0/245)0%
(0/144)src.ioslaves.http88%
(7/8)88%
(7/8)41%
(1775/4320)35%
(1304/3700)src.ioslaves.http.kcookiejar33%
(2/6)33%
(2/6)47%
(628/1330)55%
(619/1123)src.ioslaves.remote100%
(2/2)100%
(2/2)28%
(72/257)7%
(14/212)src.ioslaves.remote.kdedmodule0%
(0/2)0%
(0/2)0%
(0/12)100%
(0/0)src.ioslaves.telnet0%
(0/1)0%
(0/1)0%
(0/43)0%
(0/30)src.ioslaves.trash56%
(5/9)56%
(5/9)51%

KDE CI: Frameworks kio kf5-qt5 SUSEQt5.10 - Build # 337 - Still Unstable!

2018-07-29 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.10/337/
 Project:
Frameworks kio kf5-qt5 SUSEQt5.10
 Date of build:
Sun, 29 Jul 2018 19:16:31 +
 Build duration:
6 min 18 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 58 test(s), Skipped: 0 test(s), Total: 59 test(s)Failed: TestSuite.kiofilewidgets-kfileplacesviewtest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)65%
(258/396)65%
(258/396)53%
(31867/59888)38%
(16106/42652)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(55/55)100%
(55/55)95%
(9006/9436)51%
(3920/7700)autotests.http100%
(5/5)100%
(5/5)99%
(581/582)68%
(113/166)autotests.kcookiejar100%
(1/1)100%
(1/1)91%
(179/197)72%
(49/68)src100%
(1/1)100%
(1/1)86%
(6/7)67%
(4/6)src.core84%
(98/116)84%
(98/116)58%
(8283/14357)50%
(4638/9285)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets76%
(28/37)76%
(28/37)49%
(3893/7920)34%
(1585/4665)src.gui100%
(2/2)100%
(2/2)94%
(103/109)74%
(49/66)src.ioslaves.file100%
(5/5)100%
(5/5)52%
(527/1015)39%
(315/814)src.ioslaves.file.kauth0%
(0/2)0%
(0/2)0%
(0/106)0%
(0/65)src.ioslaves.ftp0%
(0/1)0%
(0/1)0%
(0/1364)0%
(0/1414)src.ioslaves.help0%
(0/5)0%
(0/5)0%
(0/245)0%
(0/144)src.ioslaves.http88%
(7/8)88%
(7/8)41%
(1775/4320)35%
(1304/3700)src.ioslaves.http.kcookiejar33%
(2/6)33%
(2/6)47%
(629/1330)55%
(620/1123)src.ioslaves.remote100%
(2/2)100%
(2/2)28%
(72/257)7%
(14/212)src.ioslaves.remote.kdedmodule0%
(0/2)0%
(0/2)0%
(0/12)100%
(0/0)src.ioslaves.telnet0%
(0/1)0%
(0/1)0%
(0/43)0%
(0/30)src.ioslaves.trash56%
(5/9)56%

D13805: Present error dialog when user tries to create directory named "." or ".."

2018-07-29 Thread Elvis Angelaccio
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:54298c16f37a: Present error dialog when user tries to 
create directory named . or .. (authored by tmarshall, 
committed by elvisangelaccio).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13805?vs=38050=38725

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

AFFECTED FILES
  src/filewidgets/knewfilemenu.cpp

To: tmarshall, #dolphin, #frameworks, ngraham, dfaure, elvisangelaccio
Cc: cfeck, elvisangelaccio, dfaure, tmarshall, bruns, ngraham, 
kde-frameworks-devel, michaelh, spoorun, navarromorales, firef, andrebarros, 
emmanuelp


D14468: Add QVector Definition::includedDefinitions() const

2018-07-29 Thread Dominik Haumann
dhaumann updated this revision to Diff 38723.
dhaumann added a comment.


  - Add @since tag

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14468?vs=38722=38723

BRANCH
  addIncludedDefinitions (branched from master)

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

AFFECTED FILES
  autotests/syntaxrepository_test.cpp
  src/lib/definition.cpp
  src/lib/definition.h

To: dhaumann, vkrause, cullmann
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann


D14468: Add QVector Definition::includedDefinitions() const

2018-07-29 Thread Dominik Haumann
dhaumann created this revision.
dhaumann added reviewers: vkrause, cullmann.
Restricted Application added projects: Kate, Frameworks.
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel.
dhaumann requested review of this revision.

REVISION SUMMARY
  This function returns a list of all Definition that are reference
  by IncludeRules. The returned list also contains the own Definition
  as first entry.

TEST PLAN
  make && make test

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  addIncludedDefinitions (branched from master)

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

AFFECTED FILES
  autotests/syntaxrepository_test.cpp
  src/lib/definition.cpp
  src/lib/definition.h

To: dhaumann, vkrause, cullmann
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann


D14451: Add Definition::::formats()

2018-07-29 Thread Dominik Haumann
dhaumann retitled this revision from "Add Repo::formatFromId(), 
Definition::formatFromId() and ::formats()" to "Add Definitionformats()".
dhaumann edited the summary of this revision.

REPOSITORY
  R216 Syntax Highlighting

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

To: dhaumann, cullmann, vkrause
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann


D14451: Add Repo::formatFromId(), Definition::formatFromId() and ::formats()

2018-07-29 Thread Dominik Haumann
dhaumann updated this revision to Diff 38721.
dhaumann added a comment.


  Update

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14451?vs=38720=38721

BRANCH
  addFormatGetters (branched from master)

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

AFFECTED FILES
  autotests/syntaxrepository_test.cpp
  src/lib/definition.cpp
  src/lib/definition.h

To: dhaumann, cullmann, vkrause
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann


D14451: Add Repo::formatFromId(), Definition::formatFromId() and ::formats()

2018-07-29 Thread Dominik Haumann
dhaumann updated this revision to Diff 38720.
dhaumann added a comment.


  - Update

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14451?vs=38695=38720

BRANCH
  addFormatGetters (branched from master)

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

AFFECTED FILES
  autotests/syntaxrepository_test.cpp
  src/lib/definition.cpp
  src/lib/definition.h
  src/lib/repository.cpp
  src/lib/repository.h

To: dhaumann, cullmann, vkrause
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann


D14467: Auth Support: Drop privileges if target is not owned by root

2018-07-29 Thread Chinmoy Ranjan Pradhan
chinmoyr created this revision.
chinmoyr added reviewers: dfaure, ngraham.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
chinmoyr requested review of this revision.

REVISION SUMMARY
  For actions chown, chmod and utime, process' user and group id will be
  set to target's owner and group id and for other actions it will be set
  to owner and group id target's parent directory.
  
  As a special case, for rename action the owner and group id of parent dir
  of the new file is also checked. If it's same as target's then privileges
  are dropped accordingly otherwise privileges are preserved

REPOSITORY
  R241 KIO

BRANCH
  master

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

AFFECTED FILES
  src/ioslaves/file/kauth/filehelper.cpp

To: chinmoyr, dfaure, ngraham
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


gettext makes supporting multiple languages translations in ki18n impossible

2018-07-29 Thread Albert Astals Cid
I have just realized that gettext has no way of differentiating a translation 
that is the same of the original over a translation that doesn't exist.

That is, 
  char *gettext (const char *__msgid)

Will return "foo" for the message "foo" in both cases, when the translation of 
"foo" to the target language is "foo" and when there is no translation in the 
mo catalog file for that language.

Because of this ki18n makes the assumption of "If translation is the same as 
original message, that means the translation was not found".

Let's say I undertood Spanish and hindi and i setup my languages as such.

When asking for the translation of "Hardware" (for which there is no word in 
Spanish other than "Hardware") i would get the hindi word, because ki18n would 
think that the returned "Hardware" for the Spanish translation means that 
there's no translation at all and thus i should get the Hindi version before 
defaulting to the English word.

For me it would be "sort of ok" since i do understand Hindi, but it would 
really show up as a weird thing in something that i know is 100% translated to 
Spanish.

Ideally one would be able to query gettext about whether the translation exists 
or not (basically having an option to return nullptr in the above function or 
similar), but I have not been able to find any such functionality in 
glibc/libintl itself or in any other library out there.

I know this is mostly a vague complaint, but it would be great if someone 
suggested a way to fix/workaround this other than trying to get glibc/lintl to 
add support for this :D

Cheers,
  Albert




D14449: Display used space in GiB also

2018-07-29 Thread Shubham
shubham added a comment.


  In D14449#300147 , @rkflx wrote:
  
  > In D14449#300086 , @ngraham 
wrote:
  >
  > >   Device capacity: 94.4 Gib
  > >   Device usage:==-- 24% full (71.9 GiB free, 22.5 GiB used)
  > >
  >
  >
  > That's much better. Another proposal built on top of that:
  >
  >   Device capacity: 94.4 Gib (22.5 GiB used, 71.9 GiB free)
  >-- 24% full 
  >   
  >
  > (Does not duplicate "device", shorter for improved translatability, same 
units on the same line, order of used/free matches what the bar shows.)
  >
  > Or perhaps there is a way to emphasize the most important information a bit 
more (IMO that's "71.9 GiB free")?
  
  
  yeah,agree with that , looks neat also, will do that

REPOSITORY
  R241 KIO

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

To: shubham, elvisangelaccio, ngraham, #frameworks
Cc: rkflx, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


D14449: Display used space in GiB also

2018-07-29 Thread Henrik Fehlauer
rkflx added a comment.


  In D14449#300086 , @ngraham wrote:
  
  >   Device capacity: 94.4 Gib
  >   Device usage:==-- 24% full (71.9 GiB free, 22.5 GiB used)
  >
  
  
  That's much better. Another proposal built on top of that:
  
Device capacity: 94.4 Gib (22.5 GiB used, 71.9 GiB free)
 -- 24% full 
  
  (Does not duplicate "device", shorter for improved translatability, same 
units on the same line, order of used/free matches what the bar shows.)
  
  Or perhaps there is a way to emphasize the most important information a bit 
more (IMO that's "71.9 GiB free")?

REPOSITORY
  R241 KIO

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

To: shubham, elvisangelaccio, ngraham, #frameworks
Cc: rkflx, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


D14464: Cancel privilege operation for read-only target with the current user as owner

2018-07-29 Thread Chinmoy Ranjan Pradhan
chinmoyr created this revision.
chinmoyr added reviewers: dfaure, ngraham.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
chinmoyr requested review of this revision.

REVISION SUMMARY
  If the target file (in case of chown, chmod or utime) or parent dir of target
  (for other actions) is owned by the user requesting privilege escalation then
  it's better to cancel the file operation and ask user to retry after changing
  permissions manually.

REPOSITORY
  R241 KIO

BRANCH
  master

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

AFFECTED FILES
  src/core/global.h
  src/core/job_error.cpp
  src/ioslaves/file/file_unix.cpp

To: chinmoyr, dfaure, ngraham
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D14434: add functions to access keywords

2018-07-29 Thread Alexander Semke
asemke accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  keywordlist

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

To: jpoelen, #framework_syntax_highlighting, dhaumann, asemke
Cc: vkrause, cullmann, asemke, kde-frameworks-devel, kwrite-devel, michaelh, 
kevinapavew, ngraham, bruns, demsking, sars, dhaumann


D14434: add functions to access keywords

2018-07-29 Thread Alexander Semke
asemke added a comment.


  In D14434#299932 , @dhaumann wrote:
  
  > In general looks good to me, so +1. I would like to have another +1 from 
@cullmann, @vkrause or @asemke
  
  
  +1
  
  > What I wonder is whether you really need the keyword lists from the 
Definition, or whether you want the currently active keyword lists from the 
current State / Context. I can see that both is useful.
  
  As just replied to the mailing list, both would be useful. But let's do this 
maybe step by step. With this change T4760  
will become possible. After this I'll have a look at how to simplify the 
completion logic in Cantor.

REPOSITORY
  R216 Syntax Highlighting

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

To: jpoelen, #framework_syntax_highlighting, dhaumann
Cc: vkrause, cullmann, asemke, kde-frameworks-devel, kwrite-devel, michaelh, 
kevinapavew, ngraham, bruns, demsking, sars, dhaumann


Re: how to get the list of keywords in KSyntaxHighlighting

2018-07-29 Thread Alexander Semke
Hi Dominik,

> sorry for the delay, I only now saw your mail.
> Jonathan meanwhile posted a patch that adds this:
> https://phabricator.kde.org/D14434
Yes, I saw it already. Will this make into the 5.49 release of the frameworks?


> However, pushing this further, what Kate also needs is a way go get
> all keyword lists that are matched in the current context. This allows
> for instance for context based auto completion that Kate also supports.
> 
> Would that be of interest for you as well
I think this makes sense. At the moment there is some custom logic for 
completion in Cantor's code. To refactor all this logic will take more time. 

Now I'd simply remove the manual maintanence of keywords in Cantor like it is 
done for Maxima at the moment (keywords orginally copied from Kate's code). 
For some other "backend systems" like Octave and R the keywords are obtained 
dynamically during the runtime from the backend which makes the presence of 
the actual backend system required, even if the user simply wants to _read_ a 
saved project. This is what I want to re-factor now to make https://
phabricator.kde.org/T4760 possible - the user should be able to read Cantor's 
worksheets without the backend system being present on the target system. 
Moving to KSyntaxHighlighing with the new functions added by Jonathan will 
make this possible.

So, if it's not a big problem to extend the APIs as done by Jonathan now, 
let's go for it even though Cantor is maybe the only consumer of this APIs 
now.


Thanks and Regards,
Alexander




D12709: Allow skipping the build of the KPackage install handlers when building `frameworkintegration`

2018-07-29 Thread Alexander Schlarb
tundracomp added a comment.


  I build a Qt app with „just enough“ KDE-integration for the Flatpak sandbox 
where I have no use for this KPackage stuff. I know I could just include the 
install handlers in the build and drop them afterwards, but that would require 
a build KNS (and all of *its* dependencies) as well. IMHO „only build what you 
use“ is surpreme in this case.

REPOSITORY
  R252 Framework Integration

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

To: tundracomp
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns


D12674: Mark `Phonon4Qt5` dependency as optional in CMakeLists file

2018-07-29 Thread Alexander Schlarb
tundracomp added a comment.


  Oh, I see! This wasn't obvious to me. Is this better now?

REPOSITORY
  R289 KNotifications

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

To: tundracomp
Cc: kde-frameworks-devel, apol, ltoscano, cgiboudeaux, michaelh, ngraham, bruns


D12674: Mark `Phonon4Qt5` dependency as optional in CMakeLists file

2018-07-29 Thread Alexander Schlarb
tundracomp set the repository for this revision to R289 KNotifications.
Restricted Application edited subscribers, added: kde-frameworks-devel; 
removed: Frameworks.

REPOSITORY
  R289 KNotifications

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

To: tundracomp
Cc: kde-frameworks-devel, apol, ltoscano, cgiboudeaux, michaelh, ngraham, 
bruns, #frameworks


D14449: Display used space in GiB also

2018-07-29 Thread Nathaniel Graham
ngraham requested changes to this revision.
ngraham added a reviewer: Frameworks.
ngraham added a comment.
This revision now requires changes to proceed.


  Thanks for tackling this bug! I approve of the idea here, but the 
presentation you've chosen doesn't seem ideal. How about this instead?
  
Device capacity: 94.4 Gib
Device usage:==-- 24% full (71.9 GiB free, 22.5 GiB used)

REPOSITORY
  R241 KIO

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

To: shubham, elvisangelaccio, ngraham, #frameworks
Cc: dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


Re: Pre-review CI

2018-07-29 Thread Friedrich W. H. Kossebau
Am Sonntag, 29. Juli 2018, 13:15:10 CEST schrieb Bhushan Shah:
> Hello,
> 
> So in previous email about staging repository I talked about pre-review
> CI [1]. After some time, we finally have some code in ci-tooling which can
> handle the pre-review CI for KDE repositories. And for initial pilot run
> we want to enable it for frameworks repositories.

Thanks for working on this, great to see this coming.

> We have several questions about this though,
> 
> - Should we have seperate CI jobs per platform (Linux, Windows, FreeBSD,
>   Android) for review? or just 1 job which runs the builds across all
>   platforms?

1 job means one huge build log to look at, or? In that case I would prefer 
separate jobs. Given review requests are prone to fail.

Other advantages of separate jobs: separate build job also are easier to 
compare to non-review build jobs, I would assume. And having jobs separate 
also means one gets results for any platforms, does not stop on the first 
failing?

And one can see by the job overview already on which platform there was an 
issue.

All that said though without having seen how a one-job-to-rule-them-all would 
look like, and how the others. Any chance for some samples, please?

> - Should we send out comment for failure and success? Or is it easier to
>   figure out the console log link without the comment? See linked review
>   for example [1].

[1] -> [2] here.

What do you mean exactly by "send out comment for failure and success"? More 
emails? (Please not). That example works fine with me, but not sure what the 
alternative is?

Cheers and Thanks again for improving the review system
Friedrich




D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

2018-07-29 Thread Nathaniel Graham
ngraham added a comment.


  I feel like the whole point of having these Plasma-ish wrappers is because 
they have extra features not present in the base Qt classes. If we just use 
QQC2 `TextField`s everywhere, then every one that wants a clear button (for 
example) needs to implement it for itself. That doesn't seem desirable.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: ngraham, #plasma, mart, davidedmundson
Cc: safaalfulaij, kde-frameworks-devel, michaelh, ngraham, bruns


D14451: Add Repo::formatFromId(), Definition::formatFromId() and ::formats()

2018-07-29 Thread Volker Krause
vkrause added a comment.


  The whole format id code is indeed not used here, this was purely added based 
on the requirements I got from you guys :) So feel free to change whatever is 
necessary.

REPOSITORY
  R216 Syntax Highlighting

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

To: dhaumann, cullmann, vkrause
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann


D14434: add functions to access keywords

2018-07-29 Thread Volker Krause
vkrause added a comment.


  No objection from me, assuming you have a use-case for this 
(auto-completion?).

REPOSITORY
  R216 Syntax Highlighting

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

To: jpoelen, #framework_syntax_highlighting, dhaumann
Cc: vkrause, cullmann, asemke, kde-frameworks-devel, kwrite-devel, michaelh, 
kevinapavew, ngraham, bruns, demsking, sars, dhaumann


D14457: Forward-define X509 structure

2018-07-29 Thread Luca Carlon
luc4 created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
luc4 requested review of this revision.

REVISION SUMMARY
  Forward-define X509 structure even when Qt ssl support is available.

REPOSITORY
  R239 KDELibs4Support

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

AFFECTED FILES
  src/kssl/ksslcertificate.h

To: luc4
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D14451: Add Repo::formatFromId(), Definition::formatFromId() and ::formats()

2018-07-29 Thread Christoph Cullmann
cullmann added a comment.


  I think if we have
  
  1. formats() per definition
  2. some "includedDefinitions"
  
  we can build up the needed format list per highlighting that will allow us to 
map the syntax-highlighting formats to the KTextEditor::Attribute we need 
internally and in our external API.
  
  A global lookup for id => format might then not even be needed, as we will 
internally need to lookup our wrapped KTextEditor::Attribute anyways and need 
to come up with some format->id() => internal attribute mapping after building 
that up.
  
  What is lacking in the Format ATM is an accessor for the underlying default 
style, I think, which we need, as that is exposed both in Attribute and other 
API.

REPOSITORY
  R216 Syntax Highlighting

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

To: dhaumann, cullmann, vkrause
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann


D14451: Add Repo::formatFromId(), Definition::formatFromId() and ::formats()

2018-07-29 Thread Dominik Haumann
dhaumann added a comment.


  > e.g. for the customization of coloring you want to present the list of all 
formats a highlighting has, including the included ones. (even if later the 
color of the included ones is not stored per including highlighting)
  
  The included ones behave differently in KSyntaxHighlighting and KTE: In KTE, 
the colors of included definitions can be different when included in multiple 
highlighitng files. In KSyntaxHighlighting, this is by design not possible: If 
you change "Alert" in C++, you also change Alert in Python etc... So this is 
always shared.
  
  And getting a list of all included Definitions sounds doable as well ;) That 
is probably still missing in the API. So we'd need a 
Definition::includedDefinitions (that recursively returns all Definitions it 
uses).
  
  Anything else?

REPOSITORY
  R216 Syntax Highlighting

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

To: dhaumann, cullmann, vkrause
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann


D14451: Add Repo::formatFromId(), Definition::formatFromId() and ::formats()

2018-07-29 Thread Christoph Cullmann
cullmann added a comment.


  e.g. for the customization of coloring you want to present the list of all 
formats a highlighting has, including the included ones.
  (even if later the color of the included ones is not stored per including 
highlighting)
  
  Otherwise you are right, a global lookup is all one needs, which could be 
build on-the-fly in ktexteditor, too, if we don't want to have that in the 
syntax-highlighting framework, given the id's are unique.

REPOSITORY
  R216 Syntax Highlighting

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

To: dhaumann, cullmann, vkrause
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann


D14451: Add Repo::formatFromId(), Definition::formatFromId() and ::formats()

2018-07-29 Thread Dominik Haumann
dhaumann added a comment.


  Btw, thinking more about it: I believe we do want globally unique IDs for a 
very simple reason. Think of e.g. bracket matching where multiple Definitions 
are included (e.g.:  html { . For the last '}', we want to 
find the matching attribute '{'. With globally unique attributes, this is as 
easy as compaing IDs. With Definition local IDs, we also have to check the 
Definition and the attribute.
  
  So can we at least conclude that the global IDs are what we want?
  
  With respect to #includeRules-included a Definition 'X': The IDs from 
included 'X' are the same as the IDs from 'X' itself. So the Formats are 
reused. Why do you need a list of all included IDs at all? If the IDs are 
global, you simply don't care, right?

REPOSITORY
  R216 Syntax Highlighting

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

To: dhaumann, cullmann, vkrause
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann


D14451: Add Repo::formatFromId(), Definition::formatFromId() and ::formats()

2018-07-29 Thread Christoph Cullmann
cullmann added a comment.


  I think an issue is the inclusion of definition in each other.
  I think ATM the formats are really only per definition, that means if the ids 
would be definition local, one will have clashs.
  I think ATM there is no way to get formats for definitions that got included 
at all.
  To be able to port our stuff nicely, we would need a way to have that (or to 
get some global vector of all formats for a definition including the included 
ones).

REPOSITORY
  R216 Syntax Highlighting

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

To: dhaumann, cullmann, vkrause
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann


D14451: Add Repo::formatFromId(), Definition::formatFromId() and ::formats()

2018-07-29 Thread Dominik Haumann
dhaumann added a comment.


  I just had a look into the code - the Format IDs are currently not used at 
all by KSyntaxHighlighting. Instead, all format lookups are done via the Format 
name. And that by definition means that the Format looksups are per Definition, 
since Formats from different Definitions may have the same name.
  
  In other words, if the lookup of the IDs is good enough on Definition level, 
then we can change this entirely.
  
  Maybe we can even switch the internal lookup from string-based to id-based, 
certainly not slower.

REPOSITORY
  R216 Syntax Highlighting

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

To: dhaumann, cullmann, vkrause
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann


Pre-review CI

2018-07-29 Thread Bhushan Shah
Hello,

So in previous email about staging repository I talked about pre-review
CI [1]. After some time, we finally have some code in ci-tooling which can
handle the pre-review CI for KDE repositories. And for initial pilot run
we want to enable it for frameworks repositories.

We have several questions about this though,

- Should we have seperate CI jobs per platform (Linux, Windows, FreeBSD,
  Android) for review? or just 1 job which runs the builds across all
  platforms?
- Should we send out comment for failure and success? Or is it easier to
  figure out the console log link without the comment? See linked review
  for example [1].

Looking forward to feedback.

Thanks!

[1] https://mail.kde.org/pipermail/kde-frameworks-devel/2018-June/065616.html
[2] https://phabricator.kde.org/D14438

-- 
Bhushan Shah
http://blog.bshah.in
IRC Nick : bshah on Freenode
GPG key fingerprint : 0AAC 775B B643 7A8D 9AF7 A3AC FE07 8411 7FBC E11D


signature.asc
Description: PGP signature


D14447: Sonnet: setLanguage should schedule a rehighlight if highlight is enabled

2018-07-29 Thread David Faure
dfaure closed this revision.

REPOSITORY
  R246 Sonnet

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

To: dfaure, sandsmark, mlaurent
Cc: kde-frameworks-devel, #kde_pim, michaelh, ngraham, bruns


D13808: Fix KMainWindow saving incorrect widget settings

2018-07-29 Thread David Faure
dfaure added a comment.


  https://phabricator.kde.org/D14454

REPOSITORY
  R263 KXmlGui

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

To: maxrd2, #kde_applications, dfaure, elvisangelaccio, broulik, cfeck
Cc: anthonyfieroni, marten, asturmlechner, wbauer, aacid, ngraham, 
kde-frameworks-devel, michaelh, bruns


D14434: add functions to access keywords

2018-07-29 Thread Dominik Haumann
dhaumann added subscribers: asemke, cullmann, vkrause.
dhaumann added a comment.


  In general looks good to me, so +1. I would like to have another +1 from 
@cullmann, @vkrause or @asemke
  
  What I wonder is whether you really need the keyword lists from the 
Definition, or whether you want the currently active keyword lists from the 
current State / Context. I can see that both is useful.

INLINE COMMENTS

> definition.h:175-176
>  
> +/** Returns the section names of keywords. */
> +QStringList keywordListsNames() const;
> +/** Returns a list of keywords for the specified section. */

Could we change this to:

  /**
   * Returns the names of the keyword lists of this Definition.
   * @since 5.49
   * @see keywordList()
   */
  QStringList keywordLists() const;

> definition.h:177-178
> +QStringList keywordListsNames() const;
> +/** Returns a list of keywords for the specified section. */
> +QStringList keywordList(const QString& name) const;
> +

Same here:

  /**
* Returns the list of keywords for the keyword list @p name.
* @since 5.49
* @see keywordLists()
*/
   QStringList keywordList(const QString& name) const;

REPOSITORY
  R216 Syntax Highlighting

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

To: jpoelen, #framework_syntax_highlighting, dhaumann
Cc: vkrause, cullmann, asemke, kde-frameworks-devel, kwrite-devel, michaelh, 
kevinapavew, ngraham, bruns, demsking, sars, dhaumann


D14345: Give the PlasmaComponents3 TextField the ability to have a Clear button

2018-07-29 Thread Safa Alfulaij
safaalfulaij added a comment.


  In D14345#297340 , @ngraham wrote:
  
  > 1. Does this mean that PlasmaComponents is semi or fully deprecated or 
"legacy", and we should be porting Plasma stuff to Kirigami instead?
  > 2. Since there's no Kirigami `TextField`, what do we do with this patch? Is 
there any reason why we can't improve the PC3 `TextField` to match the features 
that the PC2 `TextField` has? Or should we create a Kirigami `TextField` 
instead and port current PC2/3 clients to use it?
  
  
  https://cgit.kde.org/kirigami.git/tree/src/controls/Label.qml#n45
  
  I'll take the liberty and say that we just need to use QQC2 directly, and it 
will be themed correctly with all the enviroment variable tricks in Plasma :)

REPOSITORY
  R242 Plasma Framework (Library)

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

To: ngraham, #plasma, mart, davidedmundson
Cc: safaalfulaij, kde-frameworks-devel, michaelh, ngraham, bruns


Re: how to get the list of keywords in KSyntaxHighlighting

2018-07-29 Thread Dominik Haumann
Hi Alexander,

sorry for the delay, I only now saw your mail.
Jonathan meanwhile posted a patch that adds this:
https://phabricator.kde.org/D14434

I definitely can see that adding these functions:
QStringList keywordListsNames() const;
QStringList keywordList(const QString& name) const;
is of interest, and I don't see how it could hurt in any way.

However, pushing this further, what Kate also needs is a way go get
all keyword lists that are matched in the current context. This allows
for instance for context based auto completion that Kate also supports.

Would that be of interest for you as well

Greetings
Dominik

On Sun, Jul 15, 2018 at 11:54 AM, Alexander Semke
 wrote:
> Hi,
>
> I'd like to remove the maintenance of syntax keywords in Cantor (e.g. https://
> cgit.kde.org/cantor.git/tree/src/backends/maxima/maximakeywords.cpp) and to
> switch to KSyntaxHighlighting.
>
> Cantor uses its own highlighters and I'd need to get the list of keywords from
> KSyntaxHighlighter for this (constructor in https://cgit.kde.org/cantor.git/
> tree/src/backends/maxima/maximahighlighter.cpp). If I see it correctly,
> DefinitionData holding the keyword lists is not part of the public API. I 
> don't
> see how to get the list of keywords.
>
> Ideally, there should be something like
>
> QStringList Definition::keywordListsNames() const;
>
> to get the list of available section in the xml syntax file and
>
> QStringList Definition::keyworkList(const QString& name) const;
>
> to get the actual keywords for the specified section.
>
>
> Would this make sense or is there already another way to get the keywords?
>
>
> Regards,
> Alexander
>
>


D14447: Sonnet: setLanguage should schedule a rehighlight if highlight is enabled

2018-07-29 Thread Laurent Montel
mlaurent accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R246 Sonnet

BRANCH
  master

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

To: dfaure, sandsmark, mlaurent
Cc: kde-frameworks-devel, #kde_pim, michaelh, ngraham, bruns


D13808: Fix KMainWindow saving incorrect widget settings

2018-07-29 Thread David Faure
dfaure added a comment.


  OK I see, this is because KMail doesn't enable autosaving, but instead saves 
in the KMMainWin destructor, after the window (and all children) have been 
hidden.
  I'll try whether porting KMail to setAutoSaveSettings solves it.

REPOSITORY
  R263 KXmlGui

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

To: maxrd2, #kde_applications, dfaure, elvisangelaccio, broulik, cfeck
Cc: anthonyfieroni, marten, asturmlechner, wbauer, aacid, ngraham, 
kde-frameworks-devel, michaelh, bruns


D14451: Add Repo::formatFromId(), Definition::formatFromId() and ::formats()

2018-07-29 Thread Christoph Cullmann
cullmann added a comment.


  I am wondering: Do we really need to have unique IDs over all definitions?
  Actually I would think it would be much nicer to have all formats for one 
definition in a vector and having the index as the unique id.
  The lookup is faster and it is easier to understand. (and the 16-bit limit is 
no actual limit anymore for any realistic definition).

REPOSITORY
  R216 Syntax Highlighting

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

To: dhaumann, cullmann, vkrause
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann


D14451: Add Repo::formatFromId(), Definition::formatFromId() and ::formats()

2018-07-29 Thread Dominik Haumann
dhaumann created this revision.
dhaumann added reviewers: cullmann, vkrause.
Restricted Application added projects: Kate, Frameworks.
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel.
dhaumann requested review of this revision.

REVISION SUMMARY
  With Repository::formatFromId() it is now possible to only save the
  format ids when highlighting a line, which is a very efficient way
  to store highlighting information per text line and the way it is
  done in KTextEditor.
  
  Definition::formats() can be used to list all Formats from one
  specific Definition, e.g. for printing a syntax guide.
  
  Defintion:formatFromId() only exists for convenience and internally
  redirects to Repository::formatFromId().

TEST PLAN
  make test, added unit test

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  addFormatGetters (branched from master)

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

AFFECTED FILES
  autotests/syntaxrepository_test.cpp
  src/lib/definition.cpp
  src/lib/definition.h
  src/lib/repository.cpp
  src/lib/repository.h
  src/lib/repository_p.h

To: dhaumann, cullmann, vkrause
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann


D13808: Fix KMainWindow saving incorrect widget settings

2018-07-29 Thread Wolfgang Bauer
wbauer added a comment.


  In D13808#299791 , @ngraham wrote:
  
  > In the Bugzilla ticket, it was mentioned that KMail (and only KMail)  still 
suffers from the issue even with this patch, so it might be a KMail-specific 
issue.
  
  
  For reference: https://bugs.kde.org/show_bug.cgi?id=396339

REPOSITORY
  R263 KXmlGui

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

To: maxrd2, #kde_applications, dfaure, elvisangelaccio, broulik, cfeck
Cc: anthonyfieroni, marten, asturmlechner, wbauer, aacid, ngraham, 
kde-frameworks-devel, michaelh, bruns