D18116: Don't show document load trouble twice

dhaumann added a comment. Just for info: If openingErrorMessage is indeed not set anymore, this is a behavior change, since KTextEditor::Document::openingErrorMessage() is public API. Looking at lxr.kde.org, it seems this is not used (but ->openingError() is used, see

D18384: Allow creating directory named '~' and throw a warning before creating it.

dhaumann added inline comments. INLINE COMMENTS > shubham wrote in knewfilemenu.cpp:439 > This is faster than simple == Really? But isn't == not exactly the same? Can you elaborate? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D18384 To: shubham, ngraham Cc: dhaumann,

D18385: Build without KAuth and D-Bus on Android

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

D18386: Build without D-Bus on Android

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

D18387: Build without D-Bus on Android

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

D18388: Build without D-Bus on Android

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Lgtm. REPOSITORY R263 KXmlGui BRANCH master REVISION DETAIL https://phabricator.kde.org/D18388 To: vkrause, dhaumann Cc: dhaumann, kde-frameworks-devel, michaelh, ngraham,

D18389: Build without D-Bus on Android

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Looks good to me. Same here: do you get warnings about unused variables now? REPOSITORY R245 Solid BRANCH master REVISION DETAIL https://phabricator.kde.org/D18389 To: vkrause,

D18390: Build without D-Bus on Android

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Looks good to me. You likely get a warning about an unused variable now in KBookmarkManager::init(const QString ), since the function is empty. REPOSITORY R294 KBookmarks

D18384: Allow creating directory named '~' and throw a warning before creating it.

dhaumann added inline comments. INLINE COMMENTS > knewfilemenu.cpp:439 > KMessageBox::NoExec); > +} else if (name.operator==(QLatin1String("~"))) { > +confirmDialog->setWindowTitle(i18n("Create directory named ~?")); Please simply write name ==

D18164: Review KateGotoBar

dhaumann added a comment. Indeed I misread the screenshot - sorry for that :-) In that case, I have no objections. @cullmann your take? REVISION DETAIL https://phabricator.kde.org/D18164 To: loh.tar, #ktexteditor Cc: dhaumann, cullmann, anthonyfieroni, kwrite-devel, kde-frameworks-devel,

D18163: KateRenderer: when printing initially set the color scheme to Printing

dhaumann requested changes to this revision. dhaumann added a comment. This revision now requires changes to proceed. Looking at the printing pages, I can find the following, see screenshot. Please note the ComboBox where you can select a printing schema. In other words, this should not be

D17949: ViewPrivate: Make applyWordWrap() more comfortable

dhaumann added a comment. @loh.tar Slightly related: You may want to also have a look at this: https://phabricator.kde.org/source/ktexteditor/browse/master/src/script/data/commands/utils.js$271 In short, there is a command line command (F7) called 'rewrap' that rewraps the selection in a

D18164: Review KateGotoBar

dhaumann added a comment. I like the idea to go to next modified line up / down. I am undecided about the goto line in clipboard, since I have the feeling it adds more clutter than it helps by default: I thinkis quite fast. In addition, this leads to having "Gehe zu" twice in the ui,

D17852: ViewInternal: Fix 'Go to matching bracket' in override mode

dhaumann closed this revision. dhaumann added a comment. Closed, see commit 1e0ace28c795cbe5e88bc1f31ea37a250cd110b0 REVISION DETAIL https://phabricator.kde.org/D17852 To: loh.tar, #ktexteditor, dhaumann Cc: dhaumann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh,

D17852: ViewInternal: Fix 'Go to matching bracket' in override mode

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Good patch, and works for me. REVISION DETAIL https://phabricator.kde.org/D17852 To: loh.tar, #ktexteditor, dhaumann Cc: dhaumann, kwrite-devel, kde-frameworks-devel, #ktexteditor,

D18192: Remove unused forward declaration

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

D18148: Use HTTPS, if available, in links visible to users

dhaumann accepted this revision. This revision is now accepted and ready to land. REPOSITORY R39 KTextEditor BRANCH https-in-end-user-links (branched from master) REVISION DETAIL https://phabricator.kde.org/D18148 To: gszymaszek, #ktexteditor, dhaumann Cc: lueck, kwrite-devel,

D10446: Add KLanguageName

dhaumann added a comment. I don't think you'd have to import .desktop files. But you would need some lookup table of course to map the names. REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D10446 To: aacid, apol Cc: vkrause, dhaumann, hein,

D10446: Add KLanguageName

dhaumann added a comment. Btw, wouldn't this be a good candidate to upstream to Qt itself? REPOSITORY R265 KConfigWidgets REVISION DETAIL https://phabricator.kde.org/D10446 To: aacid, apol Cc: dhaumann, hein, kde-frameworks-devel, sitter, markg, apol, michaelh, ngraham, bruns

D18125: KateStatusBar: Add dictionary button

dhaumann added a comment. Isn't en_US too cryptic? I suggest to use the just added KLanguageName (see D10446 ). With this, you can get a nice name like "English (USA)". Alternatively, you may use QLocale("en_US").nativeLanguageName(), but I did not test

D17852: ViewInternal: Fix 'Go to matching bracket' in override mode

dhaumann added a comment. I think this change makes sense. I would like to have a unit test for this. Doing so is simple: Add a function in autotest/src/kateview_test.h/cpp and add one bracket matching call, then check the new cursor position. Then, do the same in overwrite mode.

D4716: Add some more directives to MIPS assembler highlighting

dhaumann added a comment. @arichardson friendly ping :-) In case you don't have the time, it's also OK to paste some example code in a comment here (MIT licensed, please). Then, we can proceed. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D4716 To:

D10621: Highlighting Indexer: list of suggestions

dhaumann added a comment. Herald added a project: Kate. Herald edited subscribers, added: kde-frameworks-devel, kwrite-devel; removed: Frameworks. What do we do with this? I think some of the checks are a bit too aggressive, such as folding multiple regexps into a single one: This makes

D17730: Review KateStatusBar

dhaumann added a comment. Hm, what is the current state of this patch? It was stated that it broke tests, should this be addressed now? Btw, I added a detailed comment to https://bugs.kde.org/show_bug.cgi?id=402904 that explains a proper solution to shrinking the statusbar: 1.

D17999: ViewConfig: Add option to paste at cursor position by mouse

dhaumann accepted this revision. REVISION DETAIL https://phabricator.kde.org/D17999 To: loh.tar, #ktexteditor, dhaumann Cc: cullmann, dhaumann, ngraham, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, bruns, demsking, sars

D18028: Use the nicer K_PLUGIN_CLASS_WITH_JSON

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. If that's the new way, go ahead :) REPOSITORY R39 KTextEditor BRANCH master REVISION DETAIL https://phabricator.kde.org/D18028 To: aacid, dhaumann Cc: dhaumann, kwrite-devel,

D17999: ViewConfig: Add option to paste at cursor position by mouse

dhaumann added a comment. Yes, directed @ngraham :-) REVISION DETAIL https://phabricator.kde.org/D17999 To: loh.tar, #ktexteditor, dhaumann Cc: dhaumann, ngraham, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, bruns, demsking, cullmann, sars

D17999: ViewConfig: Add option to paste at cursor position by mouse

dhaumann added a comment. Hm, didn't you switch the meaning from "Paste by mouse at cursor position" to "Paste clipboard contents at mouse cursor location"? My point is "paste ...at mouse cursor location" is the current behavior, since paste via middle mouse button moves the text cursor

D17999: ViewConfig: Add option to paste at cursor position by mouse

dhaumann added subscribers: ngraham, dhaumann. dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Good patch, I think this can go in. @ngraham: You as native English speaker, is the wording in the dialog ok? REPOSITORY R39

D17990: Update php syntax support

This revision was automatically updated to reflect the committed changes. Closed by commit R216:d85160a96653: Update php syntax support (authored by pprkut, committed by dhaumann). REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE

D17990: Update php syntax support

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Looks good to me, will integrate. REPOSITORY R216 Syntax Highlighting BRANCH php-merged REVISION DETAIL https://phabricator.kde.org/D17990 To: pprkut, dhaumann Cc: dhaumann,

D17970: Fix broken Emmet

dhaumann added inline comments. INLINE COMMENTS > dhaumann wrote in lib.js:5434 > Being a js-noob, I did some searchin the net and it seems to me that \211 is > supposed to be octal sequence. On > https://stackoverflow.com/questions/21071921 it is said that octal sequences > are optional in

D17984: Fix emmet script by using HEX instead of OCT numbers in strings

This revision was automatically updated to reflect the committed changes. Closed by commit R39:2f467b46ac6f: Fix emmet script by using HEX instead of OCT numbers in strings (authored by dhaumann). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE

D17984: Fix emmet script by using HEX instead of OCT numbers in strings

dhaumann created this revision. dhaumann added reviewers: carewolf, loh.tar. Herald added projects: Kate, Frameworks. Herald added subscribers: kde-frameworks-devel, kwrite-devel. dhaumann requested review of this revision. REVISION SUMMARY This is a follow-up commit to D17970

D17970: Fix broken Emmet

dhaumann added inline comments. INLINE COMMENTS > carewolf wrote in lib.js:5434 > Is that even valid JS? Shouldn\t it be \xHH with hexidecimal? Being a js-noob, I did some searchin the net and it seems to me that \211 is supposed to be octal sequence. On

D17730: Review KateStatusBar

dhaumann added a comment. @loh.tar I think this test uses dynamic word wrap, and therefore requires a specific width of the view. Maybe with this patch, the status bar grows and forces a different width, so that the dynamic word wrap changes and the test fails? REPOSITORY R39 KTextEditor

D17970: Fix broken Emmet

dhaumann added a comment. Btw, I committed this, but now the length of the string is 14 characters instead of 8, so this patch is certainly wrong. But better this than a completely broken emmet. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17970 To: loh.tar,

D17970: Fix broken Emmet

This revision was automatically updated to reflect the committed changes. Closed by commit R39:120e8400cad6: Fix broken Emmet (authored by dhaumann). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17970?vs=48698=48719 REVISION DETAIL

D17970: Fix broken Emmet

dhaumann added subscribers: carewolf, dhaumann. dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. @carewolf This issue was caused by the QtScript to QML migration in KTextEditor. Could you have a look or pass this on the relevant V4

D17816: Support for xattrs on kio copy/move

dhaumann added a comment. @funkybomber: Kate and KWrite stopped using QSaveFile since KF 5.50, see commit 681cafb74607d9fdb93e3bd9a90ea20b861de896 . So what you write about Kate/KWrite is not true anymore, this should

D17957: WML: fix embedded Lua code & use new default styles

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Please commit. REPOSITORY R216 Syntax Highlighting BRANCH update-wml REVISION DETAIL https://phabricator.kde.org/D17957 To: nibags, #framework_syntax_highlighting, dhaumann Cc:

D17730: Review KateStatusBar

dhaumann added a comment. @cullmann I remember that you fiddled around with the status bar quite a lot to make it pixel perfect in height, i.e. that the height of the statusbar matches the hight of e.g. the search bar. Do we loose this property by using real buttons instead of labels as

D17430: highlight token strings and delimited strings

dhaumann added a comment. Also committed, hope everything merged nicely and is correct. I had to update the folding unit test data again, but I guess that is fine. If you find any issues, please provide another patch :-) REPOSITORY R216 Syntax Highlighting REVISION DETAIL

D17430: highlight token strings and delimited strings

This revision was automatically updated to reflect the committed changes. Closed by commit R216:2a928919366d: highlight token strings and delimited strings (authored by dhaumann). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D17430?vs=48636=48638#toc REPOSITORY R216 Syntax

D17460: fixups for `extern` and `pragma`

This revision was automatically updated to reflect the committed changes. Closed by commit R216:dc79692de359: fixups for `extern` and `pragma` (authored by dhaumann). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D17460?vs=48634=48635#toc REPOSITORY R216 Syntax Highlighting CHANGES

D17430: highlight token strings and delimited strings

dhaumann added a comment. Unfortunately, this patch also has an issue: XSDError in data/syntax/d.xml, at line 413, column 53: Element context is missing required attribute lineEndContext. Can you provide an update to fix it? REPOSITORY R216 Syntax Highlighting BRANCH

D17904: Highlight CUDA .cu and .cuh files as C++

dhaumann added a comment. Thanks! More patches welcome :-) REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D17904 To: thomassc, #framework_syntax_highlighting, dhaumann Cc: dhaumann, kwrite-devel, kde-frameworks-devel, bmortimer, hase, michaelh,

D17904: Highlight CUDA .cu and .cuh files as C++

This revision was automatically updated to reflect the committed changes. Closed by commit R216:47ca116c74d2: Highlight CUDA .cu and .cuh files as C++ (authored by dhaumann). REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17904?vs=48519=48633

D17883: TypeScript & TS/JS React: improve types detection, fix float-points & other improvements/fixes

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Ok, will integrate now as well. REPOSITORY R216 Syntax Highlighting BRANCH update-typescript REVISION DETAIL https://phabricator.kde.org/D17883 To: nibags,

D17883: TypeScript & TS/JS React: improve types detection, fix float-points & other improvements/fixes

This revision was automatically updated to reflect the committed changes. Closed by commit R216:f8d8a2fbc0f5: TypeScript TS/JS React: improve types detection, fix float-points other… (authored by nibags, committed by dhaumann). REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE

D17460: fixups for `extern` and `pragma`

dhaumann added a comment. @aG0aep6G I just tried to commit this, but it does not work due to the following issue: XSDError in data/syntax/d.xml, at line 613, column 60: Element context is missing required attribute lineEndContext. Can you provide an updated patch to fix this? Or

D17939: Haskell: Highlight empty comments after 'import'

dhaumann added a comment. Committed, I also used some default styles, see patch below. You can find more of the available default styles since KF5 here: https://kate-editor.org/2014/03/07/kate-part-kf5-new-default-styles-for-better-color-schemes/ If you want, you can use these and post

D17939: Haskell: Highlight empty comments after 'import'

This revision was automatically updated to reflect the committed changes. Closed by commit R216:d173b27c4d37: Haskell: Highlight empty comments after import (authored by dhaumann). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D17939?vs=48605=48627#toc REPOSITORY R216 Syntax

D17891: WML: fix infinite loop in contexts switch & only highlight tags with valid names

This revision was automatically updated to reflect the committed changes. Closed by commit R216:c276c7003d35: WML: fix infinite loop in contexts switch only highlight tags with valid names (authored by nibags, committed by dhaumann). REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST

D17939: Haskell: Highlight empty comments after 'import'

dhaumann accepted this revision. This revision is now accepted and ready to land. REPOSITORY R216 Syntax Highlighting BRANCH import-comment REVISION DETAIL https://phabricator.kde.org/D17939 To: xialiyao, dhaumann Cc: dhaumann, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham,

D17939: Haskell: Highlight empty comments after 'import'

dhaumann added a comment. Yes, please add a test line in the autotest folder: https://github.com/KDE/syntax-highlighting/blob/master/autotests/input/highlight.hs And please also increase the version number in the language xml element. REPOSITORY R216 Syntax Highlighting REVISION

D17904: Highlight CUDA .cu and .cuh files as C++

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. The patch is fine, but when comitting, the version number should be increased. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D17904 To: thomassc,

D17891: WML: fix infinite loop in contexts switch & only highlight tags with valid names

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Good patch. More improvements are of course welcome :) Minor comment: we prefer to delete commented code, usually commented code is more confusing than helpful. Other comment:

D17241: WIP:Disable highlighting after 512 characters on a line.

dhaumann added a comment. I'm not convinced this is a good idea: Kile users writing LaTeX tend to write one paragraph into a single line. They easily hit the 512 character limit. With this patch, we'd break syntax highlighting (completely?) for Kile. Stupidly asking: Can't this be

D17817: Build without D-Bus on Android

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Given the discussion I think this should go in. REPOSITORY R289 KNotifications BRANCH arcpatch-D17817 REVISION DETAIL https://phabricator.kde.org/D17817 To: vkrause, dhaumann

D17459: SearchBar: Add Cancel button to stop long running tasks

dhaumann added inline comments. INLINE COMMENTS > cullmann wrote in katesearchbar.h:221-227 > This is a purely internal class. > It is only exported for the unit tests, no header files are installed, > therefore any change of the layout is ok ;=) Yes: as rule of thumb: only classes in the

D17857: DocumentPrivate: Don't scroll view when add auto-bracket at end of file

dhaumann added a comment. Is this related to bug https://bugs.kde.org/show_bug.cgi?id=306745 ? I once tried to fix 306745 but had to revert due to a regression, see D10054 . REPOSITORY R39 KTextEditor REVISION DETAIL

D17817: Build without D-Bus on Android

dhaumann added a comment. +1, although it will likely break quickly since this branch is not tested via CI. Then again, this is also vslid for e.g. the KSyntaxHighlighting branches that were added for standalone deployment. Another +1 would be appreciated. REPOSITORY R289

D17802: Hide API documentation of internal classes

dhaumann added a comment. Another solution is to mark the class @internal. It is still visible then on api.kde.org, though. REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D17802 To: vkrause, dhaumann Cc: dhaumann, kde-frameworks-devel, michaelh, ngraham,

D17802: Hide API documentation of internal classes

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. A bit more verbose description on why this patch is needed and a link to the generated page on api.kde.org would be useful :) PS: KTextEditor also shows hundreds of classes that

D17729: KateStatusBar: Reformatted by astyle command to follow coding style

dhaumann added a subscriber: cullmann. dhaumann added a comment. I am fine with this change. @cullmann? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17729 To: loh.tar, #ktexteditor, dhaumann Cc: cullmann, sars, kwrite-devel, kde-frameworks-devel, #ktexteditor,

D17442: KTextEditor: Tweak keyboard shortcuts to prepare for F6/Shift+F6 in Kate

dhaumann added a comment. Hm, these shortcuts exist for 15+ years... The extended shortcuts are not used widely in kde at all. Do we really address issues here? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17442 To: gregormi, #kate Cc: dhaumann, ngraham,

D17631: class Message: Use inclass member initialization

This revision was automatically updated to reflect the committed changes. Closed by commit R39:ca0f9e3cc011: class Message: Use inclass member initialization (authored by dhaumann). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17631?vs=47686=47795

D17631: class Message: Use inclass member initialization

dhaumann created this revision. dhaumann added a reviewer: loh.tar. Herald added projects: Kate, Frameworks. Herald added subscribers: kde-frameworks-devel, kwrite-devel. dhaumann requested review of this revision. REVISION SUMMARY No change in logic, just some reorganization of initialization

D17624: KTextEditor::Message: Review documentation

dhaumann closed this revision. REVISION DETAIL https://phabricator.kde.org/D17624 To: loh.tar, #ktexteditor, dhaumann Cc: dhaumann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, ngraham, bruns, demsking, cullmann, sars

D17624: KTextEditor::Message: Review documentation

dhaumann added a comment. I integrated this now, see a16d082370a44fcbae3a204bfede1db6e6dffe86 - the change also includes the rename of autoHide to autoHideDelay. The function names cannot be changed of course, since it's public API. REVISION DETAIL https://phabricator.kde.org/D17624 To:

D17624: KTextEditor::Message: Review documentation

dhaumann added a comment. Now you are mixing documentation changes with code changes. I would have preferred to have different review requests, especially since the documentation part was already reviewed. Further, I would prefer autoHideDelay instead of just delay. If we use "delay" only,

D17624: KTextEditor::Message: Review documentation

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Looks almost good, except for "optional" and a missing "be". Could you update again? INLINE COMMENTS > message.h:337 > /** > - * Optionally set an icon for this notification.

D17596: [KDirOperator] Allow renaming files from the context menu

dhaumann added inline comments. INLINE COMMENTS > elvisangelaccio wrote in kdiroperator.cpp:907-909 > I think we should just use `dialog->open()` here. I see you've copied these 3 > lines from Dolphin, but we should also fix it there (the rename dialog should > really be modal). Does

D17595: Upstream Dolphin's file rename dialog

dhaumann added inline comments. INLINE COMMENTS > renamefiledialog.h:5 > + * This program is free software; you can redistribute it and/or modify * > + * it under the terms of the GNU General Public License as published by * > + * the Free Software Foundation; either version 2 of the

D17530: improve highlighting of complex numbers

dhaumann added a comment. Well, if it's copy & pasted, it must be correct ;) REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D17530 To: cullmann, dhaumann, vkrause, #frameworks Cc: kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns,

D17530: improve highlighting of complex numbers

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Reviewing correct context popping is always not so easy - I trust this part is correct?! ;) REPOSITORY R216 Syntax Highlighting BRANCH master REVISION DETAIL

D5656: Adds method to force the reloading of a document

dhaumann added a comment. Do we really need this? Or is this a reject? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D5656 To: pedroarthurp, dfaure, brauch, dhaumann, cullmann Cc: kde-frameworks-devel, anthonyfieroni, nalvarez, kwrite-devel, hase, michaelh,

D16347: Expose KTextEditor::ViewPrivate:setInputMode(InputMode) to KTextEditor::View

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Looks good. Unfortunately the context is missing: Does it nake sense to have a // TODO KF6: make virtual? Or is that the old way? REPOSITORY R39 KTextEditor REVISION DETAIL

D17457: use STL were no implicit sharing is required

dhaumann accepted this revision. dhaumann added a comment. Looks still good to me. INLINE COMMENTS > katetextblock.cpp:64 > -// in range > -Q_ASSERT(line < m_lines.size()); > - Is there a reason why this assert is useless now? REPOSITORY R39 KTextEditor BRANCH master REVISION

D17495: [KMessageBox] Fix minimum dialog size when details are requested

dhaumann accepted this revision. This revision is now accepted and ready to land. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D17495 To: cfeck, ngraham, elvisangelaccio, dhaumann Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17460: fixups for `extern` and `pragma`

dhaumann added a comment. Could you also extend the unit test in autotest/input/highlight.d? See e.g.: https://github.com/KDE/syntax-highlighting/commit/66367f8e636623a2dbb8ee6add9c7f36683605a2 REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D17460

D17441: tune editing actions for large number of small edits

dhaumann added a comment. I guess it's ok to have this in. But please use this version at your daily work ;) I think we don't have many testers... REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17441 To: cullmann, dhaumann, #kate, loh.tar Cc: anthonyfieroni,

D17441: tune editing actions for large number of small edits

dhaumann added inline comments. INLINE COMMENTS > katetextrange.h:364 > + */ > +bool m_isCheckValidityRequired = false; > }; I would prefer an additional inline void setValidityCheckRequired(); inline bool validityCheckRequired() const; We don't need a bool in the setter, since

D17441: tune editing actions for large number of small edits

dhaumann added a comment. Hm indeed. I remember when the patch was added that added the editing positions. At that time I was not convinced about this, and now even performance problems show up. What I wonder is: Usually, these editing positions should just be the ones the user really

D17441: tune editing actions for large number of small edits

dhaumann added a comment. Just curious: Do the editing positions really show up in perf? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17441 To: cullmann, dhaumann, #kate, loh.tar Cc: kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns,

D9175: Migrate some more QRegExps to QRegularExpression

dhaumann added a comment. Herald edited subscribers, added: kde-frameworks-devel, kwrite-devel; removed: Frameworks. Curent state: This patch is only almost good to go in... :-P INLINE COMMENTS > cullmann wrote in kateview.cpp:3595 > I think the old code tried to ensure we only add

D17436: BrightScript: Remove unused keyword list 'end'

dhaumann updated this revision to Diff 47134. dhaumann added a comment. - increase version number only by one :-) REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17436?vs=47131=47134 BRANCH fix_brightscript REVISION DETAIL

D17436: BrightScript: Remove unused keyword list 'end'

dhaumann created this revision. dhaumann added reviewers: dlevin, cullmann. Herald added projects: Kate, Frameworks. Herald added subscribers: kde-frameworks-devel, kwrite-devel. dhaumann requested review of this revision. REVISION SUMMARY This warning was introduced with D17295

D17295: BrightScript: Add function/sub folding

dhaumann added inline comments. INLINE COMMENTS > brightscript.xml:290 > > - > - Since this context is not used anymore, we gen a warning while creating the highlighting index: [ 9%] Generating index.katesyntax testkatehighlightingindexer(31350)/(default)

D17413: stay in context Linkage2/Pragma2 until closing paren

This revision was automatically updated to reflect the committed changes. Closed by commit R216:66367f8e6366: stay in context Linkage2/Pragma2 until closing paren (authored by dhaumann). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D17413?vs=47069=47129#toc REPOSITORY R216 Syntax

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

dhaumann added a comment. Maybe file_save_alternatives or as Gregor once suggested file_save_variants. Or file_save_extended. Btw, do we have an issue that Kate (kateui.rc) and KTextEditor (katepart5ui.rc) use different release schedules? Does that imply issues with XMLGUI merging?

D17241: WIP:Disable highlighting for lines longer than 1024 characters.

dhaumann added a comment. Is it correct that highlighting in the document (i.e. KSyntaxHighlighting) still takes place for the entire line, and this change simply only changes the fact that KateRenderer stops using the highlighting info after 10 columns? Prior to accepting this

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,

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

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,

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

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:

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

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

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