D17137: KTextEditor: File menu: Put Save, Print and Export in submenus

dhaumann added a subscriber: cullmann. dhaumann added a comment. @cullmann You did not seem to have strong objections to this. Can you comment on this here again? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17137 To: gregormi, #kate, #kdevelop Cc: cullmann, f

D17128: DocumentPrivate: Remove comment mark when joining lines

dhaumann added a comment. As background: inserting the '*' automatically is done by the indenter in cstyle.js. Theorwtically, we could even add a joinLines function to the indenters as well and call it if it exists, and if not use the default implementation. Just an idea... Usually, the moti

D17382: KateViewInternal: Rename getMouse/Cursor() => mouse/cursorPosition() to fit coding style

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. I think is an improvement - sorry for the delay :( will integrate tomorrow. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17382 To: loh.tar, #ktexteditor,

D17413: stay in context Linkage2/Pragma2 until closing paren

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. I guess this is fine. Could you use your real name please? REPOSITORY R216 Syntax Highlighting BRANCH stay-in-context-until-closing-paren REVISION DETAIL https://phabricator.kde.

D17137: KTextEditor: File menu: Put Save, Print and Export in submenus

dhaumann added a comment. Hm, is this really a good idea? Right now, the File menu is rather flat, which is a good thing imo. Do we really want to move Save operations into a "Save Variants" sub menu? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17137 To: gre

D17084: Prolog & Lua: fix shebang

dhaumann added a comment. Btw, I think in this case the fix is correct. But sometimes, detecting a Shebang in an extra context introduces regressions when the language is used in other xml files via IncludeRules. This seems not the case here, though, so all is fine. REPOSITORY R216 Sy

D17083: Fix hidden languages in the mode menu

dhaumann added inline comments. INLINE COMMENTS > katemodemanager.cpp:103 > for (int i = 0; i < modes.size(); ++i) { > // filter out None hl, we add that later as "normal" mode > +if (modes[i].isHidden() || modes[i].name() == QLatin1String("None")) > { Now the comment is w

D16677: Add BrightScript syntax

dhaumann added a comment. Unless you have a KDE commit account you can not commit yourself. Unfortunately we cannot see on Phabricator whether you have one or not. We can integrate this patch and then it will be in the next KDE Frameworks release, i.e. early next month. REPOSITORY R216 Sy

Re: Python bindings using cppyy (was: An update on Python bindings)

... wasn't there also some python related work by Stefan? Or is that unrelated? Greetings Dominik Am Mo., 5. Nov. 2018, 16:20 hat Shaheed Haque geschrieben: > I'm afraid that there has been no progress as I am buried in "startup" > mode. I'm not sure when that might change. > > On Mon, 5 Nov 2

D16370: KTextEditor : syntax definition priority UI usability

dhaumann added subscribers: ngraham, dhaumann. dhaumann added inline comments. INLINE COMMENTS > katemodeconfigpage.cpp:95 > +// having to increase the priority of (all) those definitions. > +ui->sbPriority->setMinimum(-ui->sbPriority->maximum()); > +// make the context help a bit eas

D5802: ViewPrivate, KateSearchBar, KateVi::MatchHighlighter: use selection foreground for search highlights

dhaumann added a comment. As I see we have the following situation - the current implementation does work reasonably well - just because we cannot match a theme 100% does not imply that we have to adapt our implementation (there will always be a color in theme xy that does not properly

D16415: Creating new syntax highlighting file for Job Control Language (JCL)

dhaumann requested changes to this revision. dhaumann added a comment. This revision now requires changes to proceed. Please add a small test file and explain what this highlighting language is used for :) INLINE COMMENTS > jcl.xml:3 > + > + extensions="*.JCL;*.jcl" mimetype=""> > + katev

D16416: z/OS CLIST file syntax highlighting

dhaumann requested changes to this revision. dhaumann added a comment. This revision now requires changes to proceed. Could you explain what CLIST is used for? Please add a small test file that is also MIT licensed. INLINE COMMENTS > clist.xml:3 > + > + extensions="*.clist;*.CLIST" mimetype=

D16018: Fix align of doxygen comments in templates

dhaumann added a comment. Yes, in KDE Frameworks, there is only a master branch. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D16018 To: buschinski, #ktexteditor, dhaumann Cc: cullmann, kwrite-devel, kde-frameworks-devel, michaelh, ngraham, bruns, demsking, sar

D16018: Fix align of doxygen comments

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Well, make test works for me, and if this fixes the issue for you, I'm fine with that. Locally, I had to resolve one hunk that did not apply. Maybe you have to update the patch aga

D14948: Port scriptabletags to QJSEngine

dhaumann added a comment. Did you followup on this with @skelly ? REVISION DETAIL https://phabricator.kde.org/D14948 To: carewolf, dhaumann, skelly Cc: vkrause, skelly, dhaumann, kde-frameworks-devel

D16338: Improve R documentation highlighting

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Looks good to me, please commit. REPOSITORY R216 Syntax Highlighting BRANCH improve-rdoc REVISION DETAIL https://phabricator.kde.org/D16338 To: aaronpuchert, cullmann, dhaumann

D16300: Remove double underscore (__) from header include guards

dhaumann added a comment. Thanks! Qt decided against #pragma once. I wouldn't want to use it in KDE unless it is decided on kde-frameworks-devel. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D16300 To: kossebau, #kate, cullmann Cc: dhaumann, winterz, bcook

D8708: Introduce AbstractAnnotationItemDelegate for more control by consumer

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Feel free to push this for 5.52 (you'd have to adapt the @since 5.53 again). I only have minor comments that should not hold back this patch. One thing that pops into my eyes is tha

Re: KDE apps have missing icons when not on Breeze

Ah, just saw https://phabricator.kde.org/D14983 - so it's handled automatically. Thanks & cheers Dominik On Sun, Oct 14, 2018 at 7:53 PM Dominik Haumann wrote: > > On Sat, Sep 1, 2018 at 1:09 AM Albert Astals Cid wrote: > > > > El dissabte, 1 de setembre de 2018,

Re: KDE apps have missing icons when not on Breeze

On Sat, Sep 1, 2018 at 1:09 AM Albert Astals Cid wrote: > > El dissabte, 1 de setembre de 2018, a les 1:02:00 CEST, Dominik Haumann va > escriure: > > ...so should we revert the hack we did in Kate? > > Well, this won't be available until wuite some time in the future (

D13541: Port solid from Qt5::Widgets to Qt5::Gui

dhaumann added a comment. @graesslin ping REPOSITORY R245 Solid BRANCH gui-instead-of-widgets REVISION DETAIL https://phabricator.kde.org/D13541 To: graesslin, #frameworks, dhaumann, apol, broulik Cc: dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

Re: QtCreator-style block highlighting (and colours) in Kate(part)?

rt to KSyntaxHighlighting framework to use its Themes for instance). That said, it won't happen, except if someone provides a (good/clean!) patch... :) PS: the better place for this discussion is kwrite-de...@kde.org. Best regards Dominik René J. V. Bertin schrieb am Sa., 6. Okt. 2018, 05:15: > Do

D8708: Introduce AbstractAnnotationItemDelegate for more control by consumer

dhaumann added a comment. If Christoph accepts, I am fine with this. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D8708 To: kossebau, #kate, #kdevelop Cc: cullmann, dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, ngraham, bruns, demsking, sars

D8708: Introduce AbstractAnnotationItemDelegate for more control by consumer

dhaumann added a comment. Hm, given the size of the patch, and given it introduces new public API we cannot easily change, I think a silent +1 since noone reacts is not good enough. That said, I will not have time for another review until Oct 14th - so I would appreciate another review b

Re: QtCreator-style block highlighting (and colours) in Kate(part)?

No and no :) But feel free to create/add a color scheme called Qt Creator. Greetings Dominik René J.V. Bertin schrieb am Di., 2. Okt. 2018, 21:21: > Hi, > > I quite like Qt Creator's feature that highlights blocks (in code) by > applying a gradient of background darkening that makes code less >

D15909: Fix compile failure on Windows targets

dhaumann added a comment. Btw, can you commit yourself, or shall we push this? If so, can someone take care of this, since I am not available for the next 10 days. REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D15909 To: zrax, dhaumann Cc: dhaumann, kde-frameworks-d

D15909: Fix compile failure on Windows targets

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Lgtm. REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D15909 To: zrax, dhaumann Cc: dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D15194: Draw a line separating PlasmaComponents tab bar from its content area

dhaumann added a comment. Hm, two separator lines, even touching each other sounds like a hack. Is there no better way? REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D15194 To: ngraham, #plasma, #vdg Cc: dhaumann, romangg, abetts, kde-frameworks-

D15525: Fix rendering Aztec codes with an aspect ratio != 1

dhaumann accepted this revision. This revision is now accepted and ready to land. REPOSITORY R280 Prison BRANCH master REVISION DETAIL https://phabricator.kde.org/D15525 To: vkrause, dhaumann Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D14632: keyword rule: Spport for keywords inclusion from another language/file

dhaumann added a comment. @cullmann Can you make a decision? I trust it will be a good one ;) REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D14632 To: jpoelen, #framework_syntax_highlighting, cullmann, dhaumann Cc: kwrite-devel, vkrause, kde-frameworks-d

D15771: Logcat & SELinux: improvements for the new Solarized schemes

dhaumann accepted this revision. This revision is now accepted and ready to land. REPOSITORY R216 Syntax Highlighting BRANCH update-logcat-selinux REVISION DETAIL https://phabricator.kde.org/D15771 To: nibags, #framework_syntax_highlighting, dhaumann, cullmann Cc: kwrite-devel, kde-framew

D15780: YAML: add literal & folded block styles

dhaumann added a comment. I cannot really say much about it, since I never use YAML. Still, if the unit test works, then I am fine with this. +1 from my side. Anyone else with a +2? REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D15780 To: nibags, c

D14632: keyword rule: Spport for keywords inclusion from another language/file

dhaumann added a comment. @cullmann If the other solution is easier to maintain then I am fine with this. Still, I would like to avoid that includedDefinitions() for sass returns css just because of keywords. In fact, that was my main motivation for this solution. REPOSITORY R216 Syntax H

D14632: keyword rule: Spport for keywords inclusion from another language/file

dhaumann added a comment. Yes, and calling loadKeywords would only do work the first time. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D14632 To: jpoelen, #framework_syntax_highlighting, cullmann, dhaumann Cc: kwrite-devel, vkrause, kde-frameworks-deve

Re: Quality of Frameworks announcements

Hi, indeed I agree that the log messages should be better. Maybe blogging about how to write good log messages can already help. Besides that, there is a simple reason for this kind of messages appeared: we did big changes by replacing the entire highlighting implementation in KTextEditor by the

D15556: Fix: Email highlighting for unclosed parenthesis in Subject header

This revision was automatically updated to reflect the committed changes. Closed by commit R216:b1da54ab91e3: Fix: Email highlighting for unclosed parenthesis in Subject header (authored by dhaumann). REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D

D15556: Fix: Email highlighting for unclosed parenthesis in Subject header

dhaumann created this revision. dhaumann added a reviewer: vkrause. Herald added projects: Kate, Frameworks. Herald added subscribers: kde-frameworks-devel, kwrite-devel. dhaumann requested review of this revision. REVISION SUMMARY Unfortunately, the "Subject:" header needs to be treated indepen

D14632: keyword rule: Spport for keywords inclusion from another language/file

dhaumann added a comment. To be clear, currently we have two states: - load metadata only - load full file Instead, we could have three states: - load metadata only - load keyword lists (in addition to metadata) - load full file This is my preferred solution. ;) REPOS

D14632: keyword rule: Spport for keywords inclusion from another language/file

dhaumann added a comment. This was also my interpretation: only include the Definitions that use itemDatas / colors. What we currently do: we already support loading the language metadata without loading the rest. Then, we support loading the full XML file. What we could do: add the

D13880: [KMoreTools] Reduce menu hierarchy

dhaumann accepted this revision. This revision is now accepted and ready to land. REPOSITORY R304 KNewStuff BRANCH arcpatch-D13880 REVISION DETAIL https://phabricator.kde.org/D13880 To: nicolasfella, gregormi, dhaumann, ngraham Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D9987: Lessen log spam by not checking for existence of file with empty name

dhaumann added a comment. FYI: This now landed with 88be459559448d9d30b33f33b3ffd31fc41327c7 (cf. https://commits.kde.org/kinit/88be459559448d9d30b33f33b3ffd31fc41327c7) REPOSITORY R303 KInit REVISION DETAIL htt

D8708: Introduce AbstractAnnotationItemDelegate for more control by consumer

dhaumann added a comment. I commented on some things - I did not try this, though. What I would like to see is a comment // KF6: Merge KTextEditor::AnnotationViewInterfaceV2 into KTextEditor::AnnotationViewInterface (kossebau). For me, this comment is really important, since this tells

D13880: [KMoreTools] Reduce menu hierarchy

dhaumann added inline comments. INLINE COMMENTS > kmoretools.cpp:569 > +if (!mstruct.notInstalledServices.isEmpty()) { > +qDebug() << "notInstalledItems not empty => build 'Not > installed' section"; > +parent->addSection(i18nc("@action:inmenu", "Not installed:"))

D13880: [KMoreTools] Reduce menu hierarchy

dhaumann added a comment. Almost good enough, can you remove the forward declaration again? Just another one-line change. I then don't have any objections anymore. INLINE COMMENTS > kmoretools.h:536 > class KMoreToolsMenuBuilderPrivate; > +class KmtMenuStructure; > Please remove also thi

D15485: Paint code folding marker only for multiline code folding regions

This revision was automatically updated to reflect the committed changes. Closed by commit R39:6a35a902c969: Paint code folding marker only for multiline code folding regions (authored by dhaumann). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15485?vs=41

D15485: Paint code folding marker only for multiline code folding regions

dhaumann updated this revision to Diff 41569. dhaumann added a comment. Correct patch as discussed REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15485?vs=41567&id=41569 BRANCH fix-code-folding-markers (branched from master) REVISION DETAIL https:/

D15485: Paint code folding marker only for multiline code folding regions

dhaumann added a comment. Btw, this is a regression from the switch to the KSyntaxHighlighting framework. Maybe there is a better fix? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D15485 To: dhaumann, cullmann Cc: kwrite-devel, kde-frameworks-devel, michaelh,

D15485: Paint code folding marker only for multiline code folding regions

dhaumann edited the summary of this revision. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D15485 To: dhaumann, cullmann Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, sars, dhaumann

D15485: Paint code folding marker only for multiline code folding regions

dhaumann created this revision. dhaumann added a reviewer: cullmann. Herald added projects: Kate, Frameworks. Herald added subscribers: kde-frameworks-devel, kwrite-devel. dhaumann requested review of this revision. REVISION SUMMARY Before, a XML line like ... on the same line resulted in a

D15407: Fix OCS provider URL in about dialog

dhaumann added a reviewer: alexanderschmidt. REPOSITORY R263 KXmlGui REVISION DETAIL https://phabricator.kde.org/D15407 To: broulik, #frameworks, leinir, alexanderschmidt Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15390: Bash: fix parameter & brace expansion

dhaumann accepted this revision. dhaumann added a comment. +1, thanks. REPOSITORY R216 Syntax Highlighting BRANCH fix-bash REVISION DETAIL https://phabricator.kde.org/D15390 To: nibags, cullmann, dhaumann, #framework_syntax_highlighting Cc: kwrite-devel, kde-frameworks-devel, michaelh

D15370: Scripting: isCode() returns false for dsAlert text

This revision was automatically updated to reflect the committed changes. Closed by commit R39:af5ebba716f5: Scripting: isCode() returns false for dsAlert text (authored by dhaumann). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15370?vs=41248&id=41249 R

D15370: Scripting: isCode() returns false for dsAlert text

dhaumann created this revision. dhaumann added a reviewer: cullmann. Herald added projects: Kate, Frameworks. Herald added subscribers: kde-frameworks-devel, kwrite-devel. dhaumann requested review of this revision. REVISION SUMMARY BUG: 398393 TEST PLAN make && make test REPOSITORY R39 KT

D15358: Unit test: Check all shipped themes for completeness

This revision was automatically updated to reflect the committed changes. Closed by commit R216:58ce23f7fdb9: Unit test: Check all shipped themes for completeness (authored by dhaumann). REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15358?vs=41221

D15340: Add Solarized Light and Dark themes

dhaumann added a comment. Sure, please use kdesrc-build as described here: https://community.kde.org/Guidelines_and_HOWTOs/Build_from_source Once setup, you can build everything by invoking ./kdesrc-build You can also pass a module to build: ./kdesrc-build syntax-highlighting

D15340: Add Solarized Light and Dark themes

dhaumann added a comment. Btw, we forgot to add the themes to the Qt resource file, see also D15358 You can test the theme with the demo app in ksyntaxhighlighting: Go to the build-directory, then type ./bin/codeeditor Then, you can switch the them

D15358: Unit test: Check all shipped themes for completeness

dhaumann updated this revision to Diff 41221. dhaumann added a comment. - Merge branch 'master' into check-themes - Include Solarized color themes REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15358?vs=41219&id=41221 BRANCH check-themes (b

D15340: Add Solarized Light and Dark themes

dhaumann added a comment. See D15358 :-) REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D15340 To: acrouthamel, #kate, #framework_syntax_highlighting, dhaumann Cc: dhaumann, kwrite-devel, kde-frameworks-devel, bmortime

D15358: Unit test: Check all shipped themes for completeness

dhaumann added a comment. Example output: dh@eriador:syntax-highlighting :-) $ ./bin/theme_test * Start testing of KSyntaxHighlighting::ThemeTest * Config: Using QtTest library 5.11.1, Qt 5.11.1 (x86_64-little_endian-lp64 shared (dynamic) release build; by GCC

D15358: Unit test: Check all shipped themes for completeness

dhaumann created this revision. dhaumann added reviewers: vkrause, cullmann, acrouthamel. Herald added projects: Kate, Frameworks. Herald added subscribers: kde-frameworks-devel, kwrite-devel. dhaumann requested review of this revision. REVISION SUMMARY The added unit test makes sure - the

D15340: Add Solarized Light and Dark themes

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Looks good to me. Related: in order to have better unit testing, would you be willing to add a unit test to https://github.com/KDE/syntax-highlighting/blob/master/autotests/theme_t

D15337: Fix Solarized Light and Dark color schemes

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Looks good to me. Can you commit yourself? REPOSITORY R39 KTextEditor BRANCH fix-solarized-colors REVISION DETAIL https://phabricator.kde.org/D15337 To: acrouthamel, #kate, #kte

D15337: Fix Solarized Light and Dark color schemes

dhaumann added a comment. Solarized dark still only has 28 colors, right? Can you add the missing ones? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D15337 To: acrouthamel, #kate, #ktexteditor, #framework_syntax_highlighting Cc: dhaumann, kwrite-devel, kde-frame

D15337: Fix Solarized Light and Dark color schemes

dhaumann added inline comments. INLINE COMMENTS > katesyntaxhighlightingrc:143 > -Operator=ff93a1a1,ff93a1a1,-,-,,--- > -Others=ff859900,ff859900,-,-,,--- > -Preprocessor=ff27ae60,ff27ae60,-,-,,--- Isn't "Others" now missing? Every color theme should define 31 colors. If this was no

D15337: Fix Solarized Light and Dark color schemes

dhaumann added a comment. Thanks for working on this. Looks good to me - just a minor question about the section for Solarized light. And also related: would you also create the .theme files for KSyntaxHighlighting? INLINE COMMENTS > katesyntaxhighlightingrc:104 > # Solarized (light),

D7010: KSqueezedTextLabel: call updateGeometry() when text changes

dhaumann added a subscriber: rkflx. dhaumann added a comment. @dfaure: writing "Henrik" when he already unsubscribeb will never reach him. Ping @rkflx REVISION DETAIL https://phabricator.kde.org/D7010 To: brauch, cfeck, dfaure Cc: rkflx, dfaure, dhaumann, aacid, #frameworks, michaelh, ngra

D15274: Don't require Qt5::XmlPatterns

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Is this due to drop of own hl implementation? In any case, lgtm. REPOSITORY R39 KTextEditor BRANCH master REVISION DETAIL https://phabricator.kde.org/D15274 To: vkrause, dhauman

D15211: ::match and ::doMatch => const

dhaumann added a comment. @cullmann Can you update the docbook in kate.git/doc/ so that the documentation about writing syntax highlighting files keeps up to date also for the dynamic changes? REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D15211 To: cu

Re: KDE apps have missing icons when not on Breeze

...so should we revert the hack we did in Kate? Greetings Dominik Albert Astals Cid schrieb am Di., 28. Aug. 2018, 22:45: > El dijous, 16 d’agost de 2018, a les 15:19:36 CEST, Albert Astals Cid va > escriure: > > Missatge de Albert Vaca del dia dj., 16 d’ag. > 2018 a > > les 13:57: > > > > >

D15157: [KMountPoint] Remove AIX support

dhaumann added a comment. BTW: did you grep for other locations in kio for "_AIX"? Might make sense. REPOSITORY R241 KIO BRANCH remove_aix REVISION DETAIL https://phabricator.kde.org/D15157 To: bruns, #frameworks, dhaumann Cc: dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D15157: [KMountPoint] Remove AIX support

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. I guess this makes sense. One could add a #error in case of someone trying to build on aix, but it's probably overkill. REPOSITORY R241 KIO BRANCH remove_aix REVISION DETAIL htt

D15158: [KMountPoint] Remove Windows CE support

dhaumann accepted this revision. dhaumann added a comment. Makes sense REPOSITORY R241 KIO BRANCH remove_wince REVISION DETAIL https://phabricator.kde.org/D15158 To: bruns, #frameworks, #windows, vonreth, dhaumann Cc: dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D15134: decrease StateData space by more than 50% and half the number of needed mallocs

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Lgtm. REPOSITORY R216 Syntax Highlighting BRANCH master REVISION DETAIL https://phabricator.kde.org/D15134 To: cullmann, vkrause, dhaumann Cc: kwrite-devel, kde-frameworks-devel

D15129: improve performance of Rule::isWordDelimiter and KeywordListRule::doMatch

dhaumann added a comment. Very nice. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D15129 To: cullmann, vkrause, dhaumann Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, sars, dhaumann

D15081: replace own wildcard matcher with QRegularExpression combining all wildcards

dhaumann added a subscriber: dfaure. dhaumann added a comment. Do you need the public methods in Definition outside of KSyntaxHighlighting? In general: +1 If you want this to be in 5.50, you need to get a +2 from @vkrause and ping @dfaure to retag. INLINE COMMENTS > definition.h:19

D9987: Lessen log spam by not checking for existence of file with empty name

dhaumann added a comment. @rkflx can you please reopen so we can accept this? REPOSITORY R303 KInit REVISION DETAIL https://phabricator.kde.org/D9987 To: rkflx, #frameworks Cc: dfaure, kde-frameworks-devel, dhaumann, michaelh, ngraham, bruns

D15081: replace own wildcard matcher with QRegularExpression combining all wildcards add validator to index tool, only ? and * wildcard stuff is valid fix some syntax files that had e.g. , instead of

dhaumann added a comment. The old wildcard matcher existed for performance reasons. Did you check? PS: When doing review requests, could you please have a nice subject, and not put very long lines into its stead? :-p Especially since this line will also appear in Davids release notes? R

D9987: Lessen log spam by not checking for existence of file with empty name

dhaumann added a subscriber: dfaure. dhaumann added a comment. @rkflx I think this patch is definitely good enough to not let it go. @dfaure Can you give another +1? REPOSITORY R303 KInit REVISION DETAIL https://phabricator.kde.org/D9987 To: rkflx, #frameworks Cc: dfaure, kde-framew

D14978: Add unit test that checks Format data

This revision was automatically updated to reflect the committed changes. Closed by commit R216:e4d317da92a1: Add unit test that checks Format data (authored by dhaumann). REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14978?vs=40349&id=40351 REVI

D14978: Add unit test that checks Format data

dhaumann updated this revision to Diff 40349. dhaumann added a comment. - Verify Definition::formats() contain all formats from 1...N REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14978?vs=40172&id=40349 BRANCH check-full-formats (branched f

D14978: Add unit test that checks Format data

dhaumann added a comment. True, agreed. And it was an ugly workaround in the first place. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D14978 To: dhaumann, cullmann, vkrause Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns,

D15002: Allow to install syntax files instead of having them in a resource

dhaumann accepted this revision. This revision is now accepted and ready to land. REVISION DETAIL https://phabricator.kde.org/D15002 To: cullmann, vkrause, dhaumann Cc: dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, sars

D15002: Allow to install syntax files instead of having them in a resource

dhaumann added a comment. BTW, slightly related as idea: I think it would be nice to add a function Definition Downloader::setDownloadLocation() or similar that allows to have a custom download location. This would match nicely with Repository::addCustomSearchPath(). REPOSITORY R216 Synta

D15002: Allow to install syntax files instead of having them in a resource

dhaumann added a comment. I am fine with this change. I want a review from Volker, through . REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D15002 To: cullmann, vkrause Cc: dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bru

D14978: Add unit test that checks Format data

dhaumann added a comment. The problem likely was that Alerts.xml does not have the indentationBasedFolding attribute set. But it's needed in e.g. Python. dh@eriador:syntax :-) (check-full-formats) $ grep Alerts_indent * alert_indent.xml: coffee.xml: coffee.xml:

D14978: Add unit test that checks Format data

dhaumann updated this revision to Diff 40172. dhaumann added a comment. - Simplify code REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14978?vs=40167&id=40172 BRANCH check-full-formats (branched from master) REVISION DETAIL https://phabric

D14978: Add unit test that checks Format data

dhaumann added a comment. Yes: 1. Why don't we see Alert_indent.xml 2. Is Alert_indent.xml still needed with the new KSyntaxHighlighting framework? REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D14978 To: dhaumann, cullmann, vkrause Cc: kwrite-de

D14978: Add unit test that checks Format data

dhaumann added a reviewer: vkrause. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D14978 To: dhaumann, cullmann, vkrause Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, sars, dhaumann

D14978: Add unit test that checks Format data

dhaumann added a comment. From kate.git: commit 6c9ee3c58549d6cad46b9a42cf7407d25cdfa62e Author: Joseph Wenninger Date: Thu Sep 17 23:33:39 2009 + I'm temporarily adding an alert highlighting whic

D14978: Add unit test that checks Format data

dhaumann added a comment. Interesting: alert_indent.xml has no rules except one IncludeRules (And I am supposed to be the author?!): REPOSITORY R216 Syntax Highlighting REV

D14978: Add unit test that checks Format data

dhaumann added a comment. What fails here is Jira.xml: QDEBUG : KSyntaxHighlighting::RepositoryTest::testIncludedFormats() syntaxrepository_test(4584)/(default) ?[31m?[34mKSyntaxHighlighting::RepositoryTest::testIncludedFormats?[0m: QVector(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14,

D14978: Add unit test that checks Format data

dhaumann created this revision. dhaumann added a reviewer: cullmann. Herald added projects: Kate, Frameworks. Herald added subscribers: kde-frameworks-devel, kwrite-devel. dhaumann requested review of this revision. REVISION SUMMARY This unit test loads just one definition including its included

D14956: avoid any heap allocation for default constructed Format() as used as "invalid"

dhaumann added a comment. Btw, would it also work to point to an internal static d-pointer to avoid all the d-pointer checking? The current patch is likely fine, but it feels a bit messy... But I'd agree to this if other solutions are not feasible. REPOSITORY R216 Syntax Highlighting REVI

D14956: avoid any heap allocation for default constructed Format() as used as "invalid"

dhaumann added a comment. Unit test missing ... REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D14956 To: cullmann, vkrause Cc: dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, sars

D14952: Fix includedDefinitions, handle definition change in context switch

dhaumann added a comment. Looks ok to me. Only thing is: with takeLast() you now changed the order of the IncludedDefinitions compared to the previous version. This is of course ok, but in the color config dialog the order is now different than before. Whatever this means... I liked that Mod

D14890: Remove QSaveFile in favor of plain old file saving

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Looks reasonable to me. REPOSITORY R39 KTextEditor BRANCH nosavefile (branched from master) REVISION DETAIL https://phabricator.kde.org/D14890 To: cullmann, dhaumann, dfaure Cc:

D14897: InlineNote: Pimpl inline note data without allocs

dhaumann added a comment. Can you annotate the code here that you think is wrong? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D14897 To: dhaumann, cullmann Cc: brauch, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, demsking, cullman

D14897: InlineNote: Pimpl inline note data without allocs

dhaumann updated this revision to Diff 39911. dhaumann added a comment. - Rename InlineNote::hasFocus() to underMouse() REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14897?vs=39910&id=39911 BRANCH inline-note-data (branched from master) REVISION DET

D14897: InlineNote: Pimpl inline note data without allocs

This revision was automatically updated to reflect the committed changes. Closed by commit R39:4e279cd72493: InlineNote: Pimpl inline note data without allocs (authored by dhaumann). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14897?vs=39911&id=39912 RE

<    1   2   3   4   5   6   7   8   9   10   >