Re: Syntax-Highlighter questions
Hi,
On torsdag 20 augusti 2020 kl. 01:02:36 EEST Andreas Müller wrote:
> Hi,
>
> a while back I needed syntax-highlighting in a QML based application.
> Since I could not find some QML support for syntax-highlighter, I
> wrote a wrapper library [1]. That worked perfectly fine but now that I
> wrote an example (screenshot in [1]) unexpected behaviour pops up. The
> example is different because it can set highlighter's definition and
> theme during runtime which were set once statically in the initial
> application.
>
> Now my issue/question:
> Highlight definition: Although SyntaxHighlighter::setDefinition calls
> rehighlight(), the content of the text field remains unchanged. By
> changing the size of the window, the new highlighting is drawn. Do you
> have any suggestions what can be done to fix that?
>
> Hoping for cure :) and thanks in advance,
So the thing is that the text document does not get notified that it needs a
redraw... how is
just adding a 'onDefinitionNameChanged: textArea.append("")'
I tried it and it works, but is that too ugly?
Nice wrapper tho... :)
/Kåre
>
> Andreas
>
>
> [1] https://github.com/schnitzeltony/ksyntax-highlighting-wrapper
D29680: Fix modified line marker in kate minimap
sars accepted this revision. sars added a comment. This revision is now accepted and ready to land. Thanks for the patch! I had not noticed the problem before :) REPOSITORY R39 KTextEditor BRANCH master REVISION DETAIL https://phabricator.kde.org/D29680 To: davidedmundson, #kate, sars Cc: sars, ngraham, kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, domson, michaelh, bruns, demsking, cullmann, dhaumann
D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.
sars added a comment. I'm starting to think that we need an option for enabling/disabling this change/feature. I would not want to have the extra space between the lines, but at the same time I can see that actually not seeing the whole character is not an acceptable situation... REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D25339 To: xuetianweng, #ktexteditor, cullmann, dhaumann, #frameworks, rjvbb Cc: sars, pshinjo, rjvbb, fakefred, anthonyfieroni, kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, cblack, domson, michaelh, ngraham, bruns, demsking, cullmann, dhaumann
D26071: Avoid unwanted horizontal scrolling
This revision was automatically updated to reflect the committed changes. Closed by commit R39:92e5b0b9aac0: Avoid unwanted horizontal scrolling (authored by sars). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26071?vs=71736=72239 REVISION DETAIL https://phabricator.kde.org/D26071 AFFECTED FILES src/view/kateviewinternal.cpp To: sars, #kate, dhaumann, cullmann Cc: kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D26071: Avoid unwanted horizontal scrolling
sars added reviewers: Kate, dhaumann, cullmann. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D26071 To: sars, #kate, dhaumann, cullmann Cc: kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D26071: Avoid unwanted horizontal scrolling
sars created this revision. Herald added projects: Kate, Frameworks. Herald added subscribers: kde-frameworks-devel, kwrite-devel. sars requested review of this revision. REVISION SUMMARY It looks like the horizontal scrolling is a bit too sensitive while also scrolling vertically. This patch disables horizontal scrolling if the vertical wheel event value is larger than the horizontal. BUG: 415096 REPOSITORY R39 KTextEditor BRANCH avoid-unwanted-horizontal-scrolling (branched from master) REVISION DETAIL https://phabricator.kde.org/D26071 AFFECTED FILES src/view/kateviewinternal.cpp To: sars Cc: kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D25599: Fix plugin-metadata translations on windows
sars added a comment. Yes it works :) (I added QLocale::setDefault(QLocale(QString::fromLatin1(languageCode))); to the end of the if statement in initializeLanguages()) Hmm... For an application like Kate it is totally fine to set the language through kxmlgui, but kxmlgui is tier 3 while kcoreaddons and ki18n are tier1. Sure any application can use setDefault() to change the language of the plugin metadata, but that does not change the translation language in ki18n. I think the proper solution would be to use the same mechanism in ki18n and KCoreAddons. If we use the env variable we need to change KCoreAddons and if we want to use QLocale we need to change KI18n to use QLocale. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D25599 To: sars, aacid, vonreth Cc: kde-frameworks-devel, LeGast00n, GB_2, sdepiets, michaelh, ngraham, bruns, cullmann, kfunk
D25599: Fix plugin-metadata translations on windows
sars added a comment. I would agree that a UNIX/POSIX env var is not the most logical thing to use on Windows, but how do we ensure that we are in sync with ki18n? The usage of QLocale in ki18n looks like it is just adding extra languages to a list of languages, not actually using QLocale to determine what catalog to load. At least LANGUAGE overrides any QLocale settings. Should ki18n not be moved to using QLocale and setDefault()? Do you BTW have a hint at what the "correct" place is to use QLocale::setDefault()? REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D25599 To: sars, aacid, vonreth Cc: kde-frameworks-devel, LeGast00n, GB_2, sdepiets, michaelh, ngraham, bruns, cullmann, kfunk
D25599: Fix plugin-metadata translations on windows
sars added a comment. So the conclusion is that we are not using setDefault() yet and ki18n & gettext uses the LANGUAGE env so the logical thing, for now, is to also use LANGUAGE env here in KCoreAddons for the plugin metadata translations? REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D25599 To: sars, aacid, vonreth Cc: kde-frameworks-devel, LeGast00n, GB_2, sdepiets, michaelh, ngraham, bruns, cullmann, kfunk
D25599: Fix plugin-metadata translations on windows
sars added a comment. ki18n does not seem to use QLocale to get the language for translation, but uses LANGUAGE env. Also gettext uses the env variable. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D25599 To: sars, aacid, vonreth Cc: kde-frameworks-devel, LeGast00n, GB_2, sdepiets, michaelh, ngraham, bruns, cullmann, kfunk
D25599: Fix plugin-metadata translations on windows
sars added reviewers: aacid, vonreth. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D25599 To: sars, aacid, vonreth Cc: kde-frameworks-devel, LeGast00n, GB_2, sdepiets, michaelh, ngraham, bruns, cullmann, kfunk
D25599: Fix plugin-metadata translations on windows
sars created this revision. sars added a project: Windows. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. sars requested review of this revision. REVISION SUMMARY On Windows, the plugin metadata always follows the current region setting and any language set for the application is ignored. The reason seems to be that QLocale is not changed even if the LANGUAGE env is set. To work around this we can read the LANGUAGE env variable and use that if it is set in stead of QLocale::name(). I'm not 100% sure if this is the right solution, but this at least makes the plugin configuration page in Kate use the same language as the rest of the application if the language is manually sett. TEST PLAN Open the plugin configuration page in Kate on Windows and see that the plugin information uses the same language as the rest of the application. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D25599 AFFECTED FILES src/lib/plugin/kpluginmetadata.cpp To: sars Cc: kde-frameworks-devel, LeGast00n, GB_2, sdepiets, michaelh, ngraham, bruns, cullmann, kfunk
D22511: Minimap: Do not grab left mouse click over up/down arrows
This revision was automatically updated to reflect the committed changes. Closed by commit R39:2d941799251c: Minimap: Do not grab the left-mmouse-button-click on up/down buttons (authored by sars). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22511?vs=61921=61923 REVISION DETAIL https://phabricator.kde.org/D22511 AFFECTED FILES src/view/kateviewhelpers.cpp To: sars, brauch Cc: kwrite-devel, kde-frameworks-devel, LeGast00n, sbergeron, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D22511: Minimap: Do not grab left mouse click over up/down arrows
sars created this revision. sars added a reviewer: brauch. Herald added projects: Kate, Frameworks. Herald added subscribers: kde-frameworks-devel, kwrite-devel. sars requested review of this revision. REVISION SUMMARY The arrow buttons stopped working when we implemented "jump to position" in minimap mode. This patch excludes the scroll-bar-arrow buttons from the press-event override. TEST PLAN The up/down buttons on the minimap-scrollbar work as expected. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D22511 AFFECTED FILES src/view/kateviewhelpers.cpp To: sars, brauch Cc: kwrite-devel, kde-frameworks-devel, LeGast00n, sbergeron, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D21940: Make automatic spellcheck work after reloading a document
sars added a comment. LGTM REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D21940 To: ahmadsamir, #ktexteditor, cullmann Cc: sars, kde-frameworks-devel, kwrite-devel, LeGast00n, domson, michaelh, ngraham, bruns, demsking, cullmann, dhaumann
D17241: WIP:Disable highlighting after 512 characters on a line.
sars added a comment. I think a partially highlighted line is better than a totally non-highlighted one. And I think that the user is more likely to instinctively guess correctly why the end of the line is not highlighted than if the line is not highlighted at all. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17241 To: cullmann, vkrause, dhaumann, mwolff, sars Cc: zetazeta, mwolff, brauch, kwrite-devel, kde-frameworks-devel, LeGast00n, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17241: WIP:Disable highlighting after 512 characters on a line.
sars added a comment. I tried the patch and it improved the performance really much! :) I was able to edit a line that contained over a million characters! Unfortunately I can't comment on the code too much, as I don't know the code enough. Some general stuff: 1. Should there be a warning/information-message about why the highlighting is disabled? 2. Should the line length limit be increased? Great work! REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17241 To: cullmann, vkrause, dhaumann, mwolff, sars Cc: zetazeta, mwolff, brauch, kwrite-devel, kde-frameworks-devel, LeGast00n, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D19764: Fix Minimap with QtCurve style
sars added a comment. Yes I also have a memory that there was a bug report about it, but I can't find it now REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D19764 To: sars, brauch, dhaumann, cullmann Cc: kwrite-devel, kde-frameworks-devel, gennad, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D19764: Fix Minimap with QtCurve style
This revision was automatically updated to reflect the committed changes. Closed by commit R39:2b53012b3b50: Fix Minimap with QtCurve style (authored by sars). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D19764?vs=53895=55057 REVISION DETAIL https://phabricator.kde.org/D19764 AFFECTED FILES src/view/kateviewhelpers.cpp To: sars, brauch, dhaumann, cullmann Cc: kwrite-devel, kde-frameworks-devel, gennad, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D19764: Fix Minimap with QtCurve style
sars added reviewers: dhaumann, cullmann. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D19764 To: sars, brauch, dhaumann, cullmann Cc: kwrite-devel, kde-frameworks-devel, gennad, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D19764: Fix Minimap with QtCurve style
sars created this revision. sars added a reviewer: brauch. Herald added projects: Kate, Frameworks. Herald added subscribers: kde-frameworks-devel, kwrite-devel. sars requested review of this revision. REVISION SUMMARY The handle of the scrollbar in QtCurve is much smaller than the scrollbar width when the minimap is on and does not fill the whole width. This patch moves to always have the scrollbar-handle use the whole width (-2 pixels) TEST PLAN No visual difference with Breeze and with QtCurve the minimap scrollbar handle now uses the whole width of the scrollbar. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D19764 AFFECTED FILES src/view/kateviewhelpers.cpp To: sars, brauch Cc: kwrite-devel, kde-frameworks-devel, gennad, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D18788: Refactor KateViewInternal::mouseDoubleClickEvent(QMouseEvent *e)
sars added inline comments.
INLINE COMMENTS
> kateviewinternal.cpp:2702
> // Move cursor to end (or beginning) of selected word
> +#ifndef Q_OS_MACOS
> if (view()->selection()) {
does this even compile on OSX? Why did you change this?
> kateviewinternal.cpp:2718
> -
> -e->accept();
> -break;
Where did e->accept() / e->ignore() go? There is a reason they are there...
REPOSITORY
R39 KTextEditor
REVISION DETAIL
https://phabricator.kde.org/D18788
To: shubham, cullmann
Cc: sars, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns,
demsking, cullmann, dhaumann
D17956: DocumentPrivate: Fix broken doc links in qCWarning
sars added a comment. Adding as a warning message in the view could be another review and needs comments from others first REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17956 To: loh.tar, #ktexteditor, sars Cc: kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17956: DocumentPrivate: Fix broken doc links in qCWarning
sars accepted this revision. This revision is now accepted and ready to land. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17956 To: loh.tar, #ktexteditor, sars Cc: kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17241: WIP:Disable highlighting after 512 characters on a line.
sars added a comment. @dhaumann OK the limit is too low for Kile that is clear. Visual Studio Code is limiting the highlighting on a line to 1 characters. I tried to set the limit to 1, but that was very noticeably slow. Selecting a whole line took multiple seconds, which is probably the reason why we have had 4096 as the wrap limit ;) Is it the whole idea of limiting the highlights or just the too low limit that you object to? The main hotspots I see in my perf/hotspot profiling is RenderRangeList::advanceTo(...) in KateRenderer::decorationsForLine() and in KateRenderer::paintTextLine() the hotspot is QTextLayout::draw() (especially the one with "additionalFormats"). In both places I don't see (right now at least) very many possibilities to optimize. I think the main problem is that we draw the whole line at once even tho we only see just a tiny bit of it (when we have long lines). "Fixing" this problem would probably require that we also start to draw the lines in chunks... REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17241 To: sars, cullmann, vkrause, dhaumann, mwolff Cc: zetazeta, mwolff, brauch, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17241: WIP:Disable highlighting after 512 characters on a line.
sars marked an inline comment as done and an inline comment as not done. sars added a comment. The new limit is now limiting the number of characters highlighted on a line. It is only the rest of the line that is not highlighted. This is IMO nicer as you don't see the non-highlighted part at the beginning of the line. The highlighting makes it a bit slower, so I decreased the limit to 512. I have also made the limit into a parameter so that it can be only 100 characters for the minimap highlighting. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17241 To: sars, cullmann, vkrause, dhaumann, mwolff Cc: zetazeta, mwolff, brauch, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17241: WIP:Disable highlighting after 512 characters on a line.
sars retitled this revision from "WIP:Disable highlighting for lines longer than 1024 characters." to "WIP:Disable highlighting after 512 characters on a line.". sars edited the summary of this revision. sars edited the test plan for this revision. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17241 To: sars, cullmann, vkrause, dhaumann, mwolff Cc: zetazeta, mwolff, brauch, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17241: WIP:Disable highlighting for lines longer than 1024 characters.
sars updated this revision to Diff 48461. sars added a comment. Disable highlighting after [limit] characters on a line in stead of the whole line. REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17241?vs=47146=48461 BRANCH disable_hl2 REVISION DETAIL https://phabricator.kde.org/D17241 AFFECTED FILES src/buffer/katetextbuffer.cpp src/document/katedocument.cpp src/render/katerenderer.cpp src/render/katerenderer.h src/utils/kateconfig.cpp src/view/kateviewhelpers.cpp To: sars, cullmann, vkrause, dhaumann, mwolff Cc: zetazeta, mwolff, brauch, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17729: KateStatusBar: Reformatted by astyle command to follow coding style
sars added a comment. +1 I think this is fine, but let Christoph and Dominik decide if we want style fixing commits. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17729 To: loh.tar, #ktexteditor, dhaumann Cc: sars, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, ngraham, bruns, demsking, cullmann, dhaumann
D17693: DocumentPrivate: Treat angle bracket < and backtick ` also as "auto bracket" when we have a selection
sars added inline comments.
INLINE COMMENTS
> katedocument.cpp:117
>
> -static inline QChar matchingStartBracket(QChar c, bool withQuotes)
> +static inline QChar matchingStartBracket(const QChar )
> {
I'm not familiar with the auto-bracket code, but the change from QChar to const
QChar & does not improve the amount of copied data.
Qt documentation: "Most compilers treat it like an unsigned short."
This means that in stead of adding unsigned short to the stack you add the
64bit reference and add the tiny overhead that the reference brings with it.
It probably has no practical difference tho
REPOSITORY
R39 KTextEditor
REVISION DETAIL
https://phabricator.kde.org/D17693
To: loh.tar, #ktexteditor
Cc: sars, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh,
ngraham, bruns, demsking, cullmann, dhaumann
D17241: WIP:Disable highlighting for lines longer than 1024 characters.
sars added a comment. @zetazeta I'm not sure the highlighting length limit is worth an option as the editing of the document is not effected. I it is just a bit inconvenient that the highlighting disappears... REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17241 To: sars, cullmann, vkrause, dhaumann, mwolff Cc: zetazeta, mwolff, brauch, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17241: WIP:Disable highlighting for lines longer than 1024 characters.
sars marked 2 inline comments as done. sars added a comment. @dhaumann: You are right, the highlighting of the document still takes place and it is only KateRenderer that stops using the highlighting info for all lines longer than 1024 characters. All shorter lines are highlighted. So the length limit option is still the same: Wrapping long lines on opening a document, just 25 times longer ;) The disabled high-lighting **is** a workaround, but if we don't bring in some radical changes it will choke at some point anyways. So I would say that the target is to get the limits as high as possible. I saw that Visualstudio code uses a similar limit for highlighting lines, but I don't know if that disables the highlighting totally I found this workaround by running Kate in Hotspot ;) I can continue a bit to see if I can get the 1024 limit raised... REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17241 To: sars, cullmann, vkrause, dhaumann, mwolff Cc: zetazeta, mwolff, brauch, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17241: WIP:Disable highlighting for lines longer than 1024 characters.
sars updated this revision to Diff 47146. sars added a comment. - Use a bit different config key to force a value reset for the limit. - Fix reading the config value. - Update the line-highlight disabled info message and position. REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17241?vs=46957=47146 BRANCH disable_hl2 REVISION DETAIL https://phabricator.kde.org/D17241 AFFECTED FILES src/buffer/katetextbuffer.cpp src/document/katedocument.cpp src/render/katerenderer.cpp src/render/katerenderer.h src/utils/kateconfig.cpp To: sars, cullmann, vkrause, dhaumann, mwolff Cc: zetazeta, mwolff, brauch, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17241: WIP:Disable highlighting for lines longer than 1024 characters.
sars added a comment. The highlighting limit is now returned in a function in KateRenderer as it is used also in katedocument.cpp for the warning/information message. INLINE COMMENTS > mwolff wrote in katerenderer.cpp:400 > this style-change should be submitted independently of this code review I figured that since I'm indenting the whole section and touching the line anyways, that fixing the style does not hurt. ;) (Phabricator is not showing the white-space change) REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17241 To: sars, cullmann, vkrause, dhaumann, mwolff Cc: mwolff, brauch, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17241: WIP:Disable highlighting for lines longer than 1024 characters.
sars updated this revision to Diff 46957. sars edited the summary of this revision. sars added a comment. Add a message to inform about why the lines are not highlighted. Add a note about disabled highlighting to the wrapped lines warning. Increase the default line length limit to 100 000 characters per line. REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17241?vs=46633=46957 BRANCH disable_hl2 REVISION DETAIL https://phabricator.kde.org/D17241 AFFECTED FILES src/buffer/katetextbuffer.cpp src/document/katedocument.cpp src/render/katerenderer.cpp src/render/katerenderer.h To: sars, cullmann, vkrause, dhaumann, mwolff Cc: mwolff, brauch, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17243: Only cal updateView() in visibleRange() when endPos() is invalid.
This revision was automatically updated to reflect the committed changes. Closed by commit R39:cb63ec1ee30a: Only cal updateView() in visibleRange() when endPos() is invalid. (authored by sars). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17243?vs=46937=46956 REVISION DETAIL https://phabricator.kde.org/D17243 AFFECTED FILES src/view/kateview.cpp To: sars, cullmann, #kate, dhaumann Cc: kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17243: Only cal updateView() in visibleRange() when endPos() is invalid.
sars added a comment. OK, I have used this patch actively for a week now, and I have not noticed any regression... ;) I also now removed the temporary as it did not provide noticeable improvements. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17243 To: sars, cullmann, #kate, dhaumann Cc: kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17243: Only cal updateView() in visibleRange() when endPos() is invalid.
sars updated this revision to Diff 46937. sars edited the summary of this revision. sars added a comment. Remove unneeded temporary REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17243?vs=46500=46937 BRANCH lessUpdateView REVISION DETAIL https://phabricator.kde.org/D17243 AFFECTED FILES src/view/kateview.cpp To: sars, cullmann, #kate, dhaumann Cc: kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17241: WIP:Disable highlighting for lines longer than 1024 characters.
sars retitled this revision from "Disable highlighting for lines longer than 1024 characters." to "WIP:Disable highlighting for lines longer than 1024 characters.". REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17241 To: sars, cullmann, vkrause, dhaumann, mwolff Cc: mwolff, brauch, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17241: Disable highlighting for lines longer than 1024 characters.
sars added a comment. I agree, we need to do something about the line length limit (that only wraps the lines at the limit when opening the file). The question is: do we totally remove the option and limit or only the option and raise the limit. You can still edit the document at 200 000 characters, but it starts to get sluggish. At 500 000 it takes almost a second to insert a space (on my computer). If we keep the limit, we probably could be one of the few editor that could open and read files with millions of characters on a line. If we keep the limit what would it be? 10, 65536, 5 or what? Or do we keep the option and just raise the default limit? I'm currently working on adding a notice message about the disabled highlighting. There also seems to be a test for the long lines that needs to be updated. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17241 To: sars, cullmann, vkrause, dhaumann, mwolff Cc: mwolff, brauch, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17241: Disable highlighting for lines longer than 1024 characters.
sars updated this revision to Diff 46633. sars retitled this revision from "WIP: Disable highlighting for lines longer than 1024 characters." to "Disable highlighting for lines longer than 1024 characters.". sars edited the summary of this revision. sars added a reviewer: dhaumann. sars added a comment. (Remove the unrelated change in src/view/kateview.cpp) This version now disables the highlighting for lines longer than 1024 characters also when the line is selected. The selection highlight is shown tho. What is the way forward? Do we apply this? Should we modify the line length limit? I have now tested with a file with 97000 columns and I can start to see some slowdown but it is still usable. I think we could raise the limit to 5, but also add a notification if the file contains lines longer than 1024. REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17241?vs=46632=46633 REVISION DETAIL https://phabricator.kde.org/D17241 AFFECTED FILES src/render/katerenderer.cpp To: sars, cullmann, vkrause, dhaumann Cc: brauch, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17241: WIP: Disable highlighting for lines longer than 1024 characters.
sars updated this revision to Diff 46632. sars added a comment. Also disable highlighting in selections for lines that are longe than 1024 REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17241?vs=46499=46632 BRANCH master REVISION DETAIL https://phabricator.kde.org/D17241 AFFECTED FILES src/render/katerenderer.cpp src/view/kateview.cpp To: sars, cullmann, vkrause Cc: brauch, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17243: Only cal updateView() in visibleRange() when endPos() is invalid.
sars added a comment. Note: this is especially bad when the "on the fly spellchecking" is enabled as visibleRange() is used to "optimize" and only highlight what is currently visible. It is called multiple times. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17243 To: sars, cullmann, #kate, dhaumann Cc: kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17241: WIP: Disable highlighting for lines longer than 1024 characters.
sars added a comment. If we get it working properly we could make the limit much higher. I tried with 3 characters and it still worked very good. (except when selecting the line) REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17241 To: sars, cullmann, vkrause Cc: brauch, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17243: Only cal updateView() in visibleRange() when endPos() is invalid.
sars created this revision. sars added reviewers: cullmann, Kate, dhaumann. sars added a project: Kate. Herald added subscribers: kde-frameworks-devel, kwrite-devel. Herald added a project: Frameworks. sars requested review of this revision. REVISION SUMMARY visibleRange() has a side-effect that the visible range is updated every time the function is called. This is a big problem in createHighlights(). I created a dummy XML file that has an almost 4096 characters long line. I then used the search plugin to replace >< with >\n< to split the line. Without the patch on my computer it took 1 min 30s to split the lines. With this patch the time went down to 48s. The time is still bad, but much better ;) I wonder if the comment on line 3318 "//ensure that the view is up-to-date, otherwise 'endPos()' might fail!" warns about some corner case that I have not encountered... REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17243 AFFECTED FILES src/view/kateview.cpp To: sars, cullmann, #kate, dhaumann Cc: kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17241: WIP: Disable highlighting for lines longer than 1024 characters.
sars retitled this revision from "Disable highlighting for lines longer than 1024 characters." to "WIP: Disable highlighting for lines longer than 1024 characters.". sars edited the summary of this revision. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17241 To: sars, cullmann, vkrause Cc: brauch, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D17241: Disable highlighting for lines longer than 1024 characters.
sars created this revision. Herald added projects: Kate, Frameworks. Herald added subscribers: kde-frameworks-devel, kwrite-devel. sars requested review of this revision. REPOSITORY R39 KTextEditor BRANCH master REVISION DETAIL https://phabricator.kde.org/D17241 AFFECTED FILES src/render/katerenderer.cpp To: sars Cc: kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D14938: Force ki18n to build with the libintl.so path
sars added a comment. If I understand the comments correct it assumes that if we use a libc that already has what we need, we don't need to use a separate library for it. REPOSITORY R877 Craft Blueprints for KDE REVISION DETAIL https://phabricator.kde.org/D14938 To: sdepiets, #craft, vonreth, #frameworks, bcooksley, arichardson Cc: sars, vonreth
D14856: Search: Add workaround for missing icons in Gnome icon-theme
This revision was automatically updated to reflect the committed changes. Closed by commit R39:e63efbdb8e4c: Search: Add workaround for missing icons in Gnome icon-theme (authored by sars). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14856?vs=39772=39777 REVISION DETAIL https://phabricator.kde.org/D14856 AFFECTED FILES src/search/katesearchbar.cpp To: sars, #kate, dhaumann, broulik Cc: broulik, dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, sars
D14856: Search: Add workaround for missing icons in Gnome icon-theme
sars marked an inline comment as done. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D14856 To: sars, #kate, dhaumann Cc: broulik, dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, sars
D14856: Search: Add workaround for missing icons in Gnome icon-theme
sars updated this revision to Diff 39772.
sars added a comment.
use fromTheme("foo", fromTheme("fall-back"));
REPOSITORY
R39 KTextEditor
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D14856?vs=39766=39772
REVISION DETAIL
https://phabricator.kde.org/D14856
AFFECTED FILES
src/search/katesearchbar.cpp
To: sars, #kate, dhaumann
Cc: broulik, dhaumann, kwrite-devel, kde-frameworks-devel, michaelh,
kevinapavew, ngraham, bruns, demsking, cullmann, sars
D14856: Search: Add workaround for missing icons in Gnome icon-theme
sars edited the test plan for this revision. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D14856 To: sars Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, sars, dhaumann
D14856: Search: Add workaround for missing icons in Gnome icon-theme
sars added a reviewer: Kate. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D14856 To: sars, #kate Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, sars, dhaumann
D14856: Search: Add workaround for missing icons in Gnome icon-theme
sars created this revision. Herald added projects: Kate, Frameworks. Herald added subscribers: kde-frameworks-devel, kwrite-devel. sars requested review of this revision. TEST PLAN Install Gnome-icon-theme and start kate using that theme REPOSITORY R39 KTextEditor BRANCH master REVISION DETAIL https://phabricator.kde.org/D14856 AFFECTED FILES src/search/katesearchbar.cpp To: sars Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, sars, dhaumann
D9884: KMultiTabBar: Fix regression in conversion to new style connect()
This revision was automatically updated to reflect the committed changes. Closed by commit R236:ee34537ff55c: KMultiTabBar: Fix regression in conversion to new style connect() (authored by sars). REPOSITORY R236 KWidgetsAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9884?vs=25380=25388 REVISION DETAIL https://phabricator.kde.org/D9884 AFFECTED FILES src/kmultitabbar.cpp To: sars, #frameworks, mlaurent Cc: ltoscano
D9884: KMultiTabBar: Fix regression in conversion to new style connect()
sars closed this revision. sars added a comment. For some reason this review was not closed automatically by the commit. Closing manually. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D9884 To: sars, #frameworks, mlaurent
D9884: KMultiTabBar: Fix regression in conversion to new style connect()
sars created this revision. sars added reviewers: Frameworks, mlaurent. Restricted Application added a project: Frameworks. sars requested review of this revision. REVISION SUMMARY The conversion to new style connect introduced a bug. The old connect properly used the base class signal while the new connect connects to the derived signal which actually should be emitted as a result of the base class signal. TEST PLAN Kate can now open/close the tool views when clicking on the sidebar buttons. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D9884 AFFECTED FILES src/kmultitabbar.cpp To: sars, #frameworks, mlaurent
Re: Default location of the settings file on windows when using KXmlGuiWindow/KMainWindow
On måndag 11 december 2017 kl. 21:55:23 EET Albert Astals Cid wrote: > El dijous, 7 de desembre de 2017, a les 22:13:21 CET, Alexander Semke va > > escriure: > > Hi all, > > > > https://bugs.kde.org/show_bug.cgi?id=387626 > > we've got this problem reported and I don't see how to easily fix this. > > LabPlot's MainWindow inherits from KXmlGuiWindow. When saving the auto > > settings in KMainWindow, KSharedConfig::openConfig() is used which uses > > QStandardPaths::GenericConfigLocation for the location: > > https://api.kde.org/frameworks/kconfig/html/classKSharedConfig.html#a32820 > > 86 49f2e3f0ee895c9b11aa82205 > > > > According to the problem description in this ticket, I assume this is a > > valid request since it makes a lot of sense to me, we have to use > > QStandardPaths::AppDataLocation for the location of the config file. Do > > I need to set here KMainWindow::setAutoSettings() with a KConfig having > > the proper name and location? I'd actually expect this to be done by the > > underlying framework(s)... > > > > We read the settings in many places via KSharedConfig::openConfig() > > which also defaults to the same rc file under > > QStandardPaths::GenericConfigLocation. For all our custom and > > application specific settings we can of course provide AppDataLocation > > explicitely but this also looks to me like not the most optimal solution > > since we'd need to touch the code at many places. How is this handled in > > other KF5-applications? Can somebody share some best practices here? > > You may try the kde windows list if noone answers here. Actually I think kde-frameworks-devel would be a better place to take this up. (CC:ing) One problem is that we have have used GenericConfigLocation long before AppDataLocation was introduced and there would be backwards compatibility problems if we directly would just switch to AppDataLocation. Regards, Kåre > > Cheers, > Albert > > > Thanks and Regards, > > Alexander
D7884: - Increase size of trailing mark
sars added a comment. I'm OK with an option for this. The only problem, with the option in the screen-shot, is that it could be interpreted as also effecting the tabulator mark. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D7884 To: helio, mwolff, kfunk Cc: cullmann, anthonyfieroni, dhaumann, sars, #frameworks
D7884: - Increase size of trailing mark
sars added a comment. I think "spaceWidth() / 2" goes a little bit too far. Shouldn't spaceWidth() fix the HiDPI problem? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D7884 To: helio, mwolff, kfunk Cc: sars, #frameworks, cullmann, dhaumann
D7443: Fix compilation on windows with editorconfig-c-core available
sars created this revision. Restricted Application added subscribers: Frameworks, kwrite-devel. Restricted Application added a project: Frameworks. REVISION SUMMARY On Windows the library was found during config, but not during compilation without the variable EditorConfig_LIBRARIES. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D7443 AFFECTED FILES src/CMakeLists.txt To: sars, cullmann, #kate Cc: kwrite-devel, #frameworks
D6086: Left-click mini-map to jump to clicked area
sars added a comment. I think I fixed it. The comment about the dragging was only partially correct. The press must be inside the slider for dragging, so when the initial press is outside or not translated from the mini-map coordinates, the dragging does not start. My solution is to first move to the desired place then forward the translated click to the QScrollBar. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D6086 To: sars, #ktexteditor, #kate, dhaumann, kfunk Cc: kfunk, broulik, kwrite-devel, #frameworks
D6086: Left-click mini-map to jump to clicked area
This revision was automatically updated to reflect the committed changes. Closed by commit R39:7bb1f434afaf: Jump to the clicked scrollbar position when minim-map is enabled. (authored by sars). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6086?vs=15180=15182 REVISION DETAIL https://phabricator.kde.org/D6086 AFFECTED FILES src/view/kateviewhelpers.cpp src/view/kateviewhelpers.h To: sars, #ktexteditor, #kate, dhaumann, kfunk Cc: kfunk, broulik, kwrite-devel, #frameworks
D6086: Left-click mini-map to jump to clicked area
sars updated this revision to Diff 15180. sars added a comment. Use qBound() + remove one set of unneeded parenthesis REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6086?vs=15128=15180 REVISION DETAIL https://phabricator.kde.org/D6086 AFFECTED FILES src/view/kateviewhelpers.cpp src/view/kateviewhelpers.h To: sars, #ktexteditor, #kate, dhaumann, kfunk Cc: kfunk, broulik, kwrite-devel, #frameworks
D6086: Left-click mini-map to jump to clicked area
sars added a comment. Reasoning behind the change of behavior: - With mini-map it is seems a bit more logical to jump to the clicked place in stead of "searching your way there". (You already know where you want to go). - Page Up/Down and mouse scroll can still be used. - The middle click action is not very well known. - On Windows middle-click does nothing. - At least VisualStudio and Sublime does it like this. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D6086 To: sars, #ktexteditor, #kate, dhaumann Cc: broulik, kwrite-devel, #frameworks
D6086: Left-click mini-map to jump to clicked area
sars created this revision. Restricted Application added subscribers: Frameworks, kwrite-devel. Restricted Application added a project: Frameworks. REVISION SUMMARY This patch implements the jump to clicked position in "show-minimap" mode for the scrollbar. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D6086 AFFECTED FILES src/view/kateviewhelpers.cpp src/view/kateviewhelpers.h To: sars, #ktexteditor, #kate, dhaumann Cc: kwrite-devel, #frameworks
D5102: Add scroll-barmarks also to the built-in search
sars closed this revision. sars added a comment. https://commits.kde.org/ktexteditor/1bd9d474763ca891ef67c736398289cfd61bf21f REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D5102 To: sars, #kate, #ktexteditor, dhaumann Cc: mwolff, dhaumann, kwrite-devel, #frameworks
Re: kioslave nogui
Hi,
What version of Qt are you using?
I think this might be a Qt bug that you are encountering.
https://www.mail-archive.com/kde-windows@kde.org/msg07290.html
There was a patch for KIO to workaround the problem, but it was rejected
because the proper fix
is in Qt 5.8.
For building Kate on windows I check the Qt version and apply the attached
patch if the Qt
version is less than 5.8
/Kåre
On fredag 31 mars 2017 kl. 13:01:22 EEST Harald Sitter wrote:
> Hola,
>
> I am tyring to run filelight on windows and encountered a really
> strange behavior in KIO. I am building KIO with KIO_FORK_SLAVES=ON as
> that seems to make the most sense on Windows. That will fork the
> kioslave helper binary to run the slaves rather than going through
> kdeinit. The problem is that the kioslave binary is built with
> ecm_mark_nongui_executable which sets `WIN32_EXECUTABLE FALSE`.
>
> Maybe I am being daft, but the way I understand it WIN32_EXECUTABLE
> being set to false means that the program will always run in a
> terminal. i.e. it is a cmdline program. It is like when one sets
> `Terminal=true` in a freedesktop desktop file.
>
> Long story short: this is what I get due to ecm_mark_nongui_executable
> http://i.imgur.com/sHeyyIp.png
>
> I am not sure about the OSX implication of ecm_mark_nongui_executable,
> but at least from a Windows point of view it seems to make no sense as
> it brings up this utterly useless command window. I've confirmed that
> not calling ecm_mark_nongui_executable in fact gets rid of the window.
>
> I guess I am asking for confirmation that we can drop the call
> entirely and if not if we there is any downside to dropping it for
> windows.
>
> Cheers,
> HS
diff --git a/src/ioslaves/http/CMakeLists.txt b/src/ioslaves/http/CMakeLists.txt
index 76a8e2800b84c312431cc1996ac81d1ef6fb5cfc..144119d0b306d974e1c6f40745575104d8a74d48 100644
--- a/src/ioslaves/http/CMakeLists.txt
+++ b/src/ioslaves/http/CMakeLists.txt
@@ -38,7 +38,7 @@ set(kio_http_cache_cleaner_SRCS
)
-add_executable(kio_http_cache_cleaner ${kio_http_cache_cleaner_SRCS})
+add_executable(kio_http_cache_cleaner WIN32 ${kio_http_cache_cleaner_SRCS})
target_link_libraries(kio_http_cache_cleaner
Qt5::DBus
diff --git a/src/ioslaves/http/kcookiejar/CMakeLists.txt b/src/ioslaves/http/kcookiejar/CMakeLists.txt
index 7b4778d1f67c1ad9f9edcaa4692b39ee6fe3f365..7d33c52a34619be8667e43baaabbeae99559bb99 100644
--- a/src/ioslaves/http/kcookiejar/CMakeLists.txt
+++ b/src/ioslaves/http/kcookiejar/CMakeLists.txt
@@ -5,8 +5,7 @@ set_source_files_properties(${kcookieserver_xml} PROPERTIES INCLUDE "kcookiejar_
qt5_add_dbus_interfaces(kcookiejar_SRCS ${kcookieserver_xml})
set(kcookiejar_SRCS ${kcookiejar_SRCS} main.cpp)
-add_executable( kcookiejar5 ${kcookiejar_SRCS})
-ecm_mark_nongui_executable(kcookiejar5)
+add_executable(kcookiejar5 WIN32 ${kcookiejar_SRCS})
target_link_libraries( kcookiejar5
Qt5::DBus
diff --git a/src/kioexec/CMakeLists.txt b/src/kioexec/CMakeLists.txt
index 91284a3a61b86770b4d1939da52d256840803608..ef63d39eca1cf7132c4b8896276bb68cbb29c5cc 100644
--- a/src/kioexec/CMakeLists.txt
+++ b/src/kioexec/CMakeLists.txt
@@ -1,9 +1,7 @@
-add_executable(kioexec main.cpp)
+add_executable(kioexec WIN32 main.cpp)
configure_file(config-kioexec.h.cmake ${CMAKE_CURRENT_BINARY_DIR}/config-kioexec.h)
-ecm_mark_nongui_executable(kioexec)
-
target_link_libraries(kioexec
Qt5::Widgets
KF5::I18n
diff --git a/src/kioslave/CMakeLists.txt b/src/kioslave/CMakeLists.txt
index e02febd380b268c596e8ecc3b745b6f50993ab4e..89c10c0c9c800b605a835182af4ffa11f6e7803b 100644
--- a/src/kioslave/CMakeLists.txt
+++ b/src/kioslave/CMakeLists.txt
@@ -1,5 +1,4 @@
-add_executable(kioslave kioslave.cpp)
-ecm_mark_nongui_executable(kioslave)
+add_executable(kioslave WIN32 kioslave.cpp)
target_link_libraries(kioslave
KF5::KIOCore
diff --git a/src/kpac/CMakeLists.txt b/src/kpac/CMakeLists.txt
index fc5989714480ca49b5bd72e1c7b458b26bd0d9bc..b8f5e17dd0878e03ccd9a3fa6f076368d1689972 100644
--- a/src/kpac/CMakeLists.txt
+++ b/src/kpac/CMakeLists.txt
@@ -44,8 +44,7 @@ endif()
### next target ###
-add_executable(kpac_dhcp_helper kpac_dhcp_helper.c)
-ecm_mark_nongui_executable(kpac_dhcp_helper)
+add_executable(kpac_dhcp_helper WIN32 kpac_dhcp_helper.c)
if (HAVE_NSL_LIBRARY)
# Assume Solaris
D5102: Add scroll-barmarks also to the built-in search
sars added a comment. I searched through the kdevelop repo and found marks 8, 21-27 in use + the "reserved" for Bookmark,... So for KDevelop it looks OK, but we have other users also. Should I add "SearchMatch = markType32" to the MarkTypes enum to reserve it? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D5102 To: sars, #kate, #ktexteditor, dhaumann Cc: dhaumann, kwrite-devel, #frameworks
D5102: Add scroll-barmarks also to the built-in search
sars created this revision. Restricted Application added subscribers: Frameworks, kwrite-devel. Restricted Application added a project: Frameworks. REVISION SUMMARY This patch adds scroll-bar-marks to the matches of "Find all" in the advanced search bar TEST PLAN find all results in multiple marks and then pressing "ESC" removes them along with the highlights. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D5102 AFFECTED FILES src/search/katesearchbar.cpp To: sars, #kate, #ktexteditor Cc: kwrite-devel, #frameworks
Re: Review Request 128191: Let KCapacityBar be usable in Qt Designer
> On June 16, 2016, 4:47 p.m., Kåre Särs wrote: > > I'm getting this error on Windows with MSVC 2015 after this commit. > > > > ...etsaddons\src\kcapacitybar.h(234): error C2668: > > 'KCapacityBar::KCapacityBar': ambiguous call to overloaded function > > ...etsaddons\src\kcapacitybar.h(80): note: could be > > 'KCapacityBar::KCapacityBar(KCapacityBar::DrawTextMode,QWidget *)' > > ...etsaddons\src\kcapacitybar.h(71): note: or > > 'KCapacityBar::KCapacityBar(QWidget *)' > > ...etsaddons\src\kcapacitybar.h(234): note: while trying to match the > > argument list '()' > > Elvis Angelaccio wrote: > Could this be a bug in MSVC? Removing the default argument from the new one with only one argument could maybe solve the problem for all cases. - Kåre --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128191/#review96599 --- On June 16, 2016, 1:10 p.m., Elvis Angelaccio wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128191/ > --- > > (Updated June 16, 2016, 1:10 p.m.) > > > Review request for KDE Frameworks and Christoph Feck. > > > Repository: kwidgetsaddons > > > Description > --- > > Currently KCapacityBar doesn't seem to be usable from Qt Designer (unless I'm > doing something wrong). > The class has only an explicit multi-argument constructor, but Qt Designer > generates something like: > > ```c++ > capacityBar = new KCapacityBar(widget_3); > ``` > > which fails to compile because there is no explicit conversion from > `QWidget*` to `KCapacityBar::DrawTextMode`: > > ``` > error: no matching constructor for initialization of 'KCapacityBar' > capacityBar = new KCapacityBar(widget_3); > ^ > /usr/include/KF5/KWidgetsAddons/kcapacitybar.h:44:29: note: candidate > constructor (the implicit copy constructor) not viable: no known conversion > from > 'QWidget *' to 'const KCapacityBar' for 1st argument > class KWIDGETSADDONS_EXPORT KCapacityBar > ^ > /usr/include/KF5/KWidgetsAddons/kcapacitybar.h:72:14: note: candidate > constructor not viable: no known conversion from 'QWidget *' to > 'KCapacityBar::DrawTextMode' for 1st argument > explicit KCapacityBar(DrawTextMode drawTextMode = DrawTextOutline, > QWidget *parent = 0); > ``` > > This patch simply adds another constructor with a single argument (the > pointer to the parent). > > > Diffs > - > > src/kcapacitybar.h 1a1f2e60f415904d6cf603a4b4e0cae1c543169a > src/kcapacitybar.cpp ecd2c13feb88f5c9e92086639bff48c6d0527779 > src/kmultitabbar.h 068f0980827e6806d7cb2a17e8b261f7e1620204 > src/kmultitabbar.cpp bc58b81cbe4d98043762a36a43b4777ada30734f > > Diff: https://git.reviewboard.kde.org/r/128191/diff/ > > > Testing > --- > > KCapacityBar can now be used in a Qt Designer custom widget. > > > Thanks, > > Elvis Angelaccio > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 128191: Let KCapacityBar be usable in Qt Designer
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128191/#review96599 --- I'm getting this error on Windows with MSVC 2015 after this commit. ...etsaddons\src\kcapacitybar.h(234): error C2668: 'KCapacityBar::KCapacityBar': ambiguous call to overloaded function ...etsaddons\src\kcapacitybar.h(80): note: could be 'KCapacityBar::KCapacityBar(KCapacityBar::DrawTextMode,QWidget *)' ...etsaddons\src\kcapacitybar.h(71): note: or 'KCapacityBar::KCapacityBar(QWidget *)' ...etsaddons\src\kcapacitybar.h(234): note: while trying to match the argument list '()' - Kåre Särs On June 16, 2016, 1:10 p.m., Elvis Angelaccio wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128191/ > --- > > (Updated June 16, 2016, 1:10 p.m.) > > > Review request for KDE Frameworks and Christoph Feck. > > > Repository: kwidgetsaddons > > > Description > --- > > Currently KCapacityBar doesn't seem to be usable from Qt Designer (unless I'm > doing something wrong). > The class has only an explicit multi-argument constructor, but Qt Designer > generates something like: > > ```c++ > capacityBar = new KCapacityBar(widget_3); > ``` > > which fails to compile because there is no explicit conversion from > `QWidget*` to `KCapacityBar::DrawTextMode`: > > ``` > error: no matching constructor for initialization of 'KCapacityBar' > capacityBar = new KCapacityBar(widget_3); > ^ > /usr/include/KF5/KWidgetsAddons/kcapacitybar.h:44:29: note: candidate > constructor (the implicit copy constructor) not viable: no known conversion > from > 'QWidget *' to 'const KCapacityBar' for 1st argument > class KWIDGETSADDONS_EXPORT KCapacityBar > ^ > /usr/include/KF5/KWidgetsAddons/kcapacitybar.h:72:14: note: candidate > constructor not viable: no known conversion from 'QWidget *' to > 'KCapacityBar::DrawTextMode' for 1st argument > explicit KCapacityBar(DrawTextMode drawTextMode = DrawTextOutline, > QWidget *parent = 0); > ``` > > This patch simply adds another constructor with a single argument (the > pointer to the parent). > > > Diffs > - > > src/kcapacitybar.h 1a1f2e60f415904d6cf603a4b4e0cae1c543169a > src/kcapacitybar.cpp ecd2c13feb88f5c9e92086639bff48c6d0527779 > src/kmultitabbar.h 068f0980827e6806d7cb2a17e8b261f7e1620204 > src/kmultitabbar.cpp bc58b81cbe4d98043762a36a43b4777ada30734f > > Diff: https://git.reviewboard.kde.org/r/128191/diff/ > > > Testing > --- > > KCapacityBar can now be used in a Qt Designer custom widget. > > > Thanks, > > Elvis Angelaccio > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 128187: Add a program to convert symbolically linked files to qrc aliases
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128187/ --- (Updated June 16, 2016, 4:02 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks, Andreas Kainz, David Faure, and Kevin Funk. Repository: breeze-icons Description --- On windows git converts symbolic links to text files with links to the actual file. This added application modifies the .qrc to use aliases for the links. This change also adds an option to not install the icons. Diffs - CMakeLists.txt 08cd1a0 icons-dark/CMakeLists.txt 5d7e6f8 icons/CMakeLists.txt bd4c048 qrcAlias.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/128187/diff/ Testing --- Compiled on windows with BINARY_ICONS_RESOURCE and SKIP_INSTALL_ICONS enabled. We get the icon .rcc file and the icons where not installed Thanks, Kåre Särs ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 128187: Add a program to convert symbolically linked files to qrc aliases
> On June 16, 2016, 1:14 p.m., David Faure wrote: > > main.cpp, line 29 > > <https://git.reviewboard.kde.org/r/128187/diff/1/?file=468769#file468769line29> > > > > Recursion? Is there a risk of infinite recursion here? Yes there is a risk of recursion if the symbolic links also are. Should we add a check or can we assume that the links are sane? - Kåre --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128187/#review96587 --- On June 16, 2016, 3:26 p.m., Kåre Särs wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128187/ > --- > > (Updated June 16, 2016, 3:26 p.m.) > > > Review request for KDE Frameworks, Andreas Kainz, David Faure, and Kevin Funk. > > > Repository: breeze-icons > > > Description > --- > > On windows git converts symbolic links to text files with links to > the actual file. This added application modifies the .qrc to use > aliases for the links. > > This change also adds an option to not install the icons. > > > Diffs > - > > CMakeLists.txt 08cd1a0 > icons-dark/CMakeLists.txt 5d7e6f8 > icons/CMakeLists.txt bd4c048 > qrcAlias.cpp PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/128187/diff/ > > > Testing > --- > > Compiled on windows with BINARY_ICONS_RESOURCE and SKIP_INSTALL_ICONS enabled. > > We get the icon .rcc file and the icons where not installed > > > Thanks, > > Kåre Särs > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 128187: Add a program to convert symbolically linked files to qrc aliases
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128187/ --- (Updated June 16, 2016, 3:26 p.m.) Review request for KDE Frameworks, Andreas Kainz, David Faure, and Kevin Funk. Changes --- Fix all issues except the recursion plus move the temporary .qrc file out of the resource directory to not pollute the .rcc Repository: breeze-icons Description --- On windows git converts symbolic links to text files with links to the actual file. This added application modifies the .qrc to use aliases for the links. This change also adds an option to not install the icons. Diffs (updated) - CMakeLists.txt 08cd1a0 icons-dark/CMakeLists.txt 5d7e6f8 icons/CMakeLists.txt bd4c048 qrcAlias.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/128187/diff/ Testing --- Compiled on windows with BINARY_ICONS_RESOURCE and SKIP_INSTALL_ICONS enabled. We get the icon .rcc file and the icons where not installed Thanks, Kåre Särs ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 128187: Add a program to convert symbolically linked files to qrc aliases
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128187/ --- Review request for KDE Frameworks, Andreas Kainz and David Faure. Repository: breeze-icons Description --- On windows git converts symbolic links to text files with links to the actual file. This added application modifies the .qrc to use aliases for the links. This change also adds an option to not install the icons. Diffs - CMakeLists.txt 08cd1a0 icons-dark/CMakeLists.txt 5d7e6f8 icons/CMakeLists.txt bd4c048 main.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/128187/diff/ Testing --- Compiled on windows with BINARY_ICONS_RESOURCE and SKIP_INSTALL_ICONS enabled. We get the icon .rcc file and the icons where not installed Thanks, Kåre Särs ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127896: make dbus optional on osx: kauth
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127896/#review95681 --- CMakeLists.txt (line 12) <https://git.reviewboard.kde.org/r/127896/#comment64789> Why is this not: option(DISABLE_DBUS "Remove..." OFF) ? For the rest I'm happy :) - Kåre Särs On May 20, 2016, 9:13 p.m., Nick Shaforostoff wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127896/ > --- > > (Updated May 20, 2016, 9:13 p.m.) > > > Review request for KDE Software on Mac OS X, KDE Frameworks, David Edmundson, > and Martin Gräßlin. > > > Repository: kauth > > > Description > --- > > this is the first patch to make kde frameworks build (and then work) without > dbus. > > this will allow homebrew users use precompiled vanilla Qt to build kde apps > on osx. as dbus is not a common service in osx world, kde apps on osx should > use native means for interprocess communication instead -- this will make > them better citizens in osx ecosystem. > > > Diffs > - > > CMakeLists.txt 48dc2d9 > autotests/BackendsManager.cpp 59675b3 > autotests/CMakeLists.txt b53d760 > autotests/HelperTest.cpp 8050a06 > src/CMakeLists.txt 1b6930d > src/ConfigureChecks.cmake d46761a > > Diff: https://git.reviewboard.kde.org/r/127896/diff/ > > > Testing > --- > > compiles fine on osx, compiles fine on linux, tests on linux still pass. > > > Thanks, > > Nick Shaforostoff > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127911: Add a CMake option to build binary Qt resource out of icons dir.
> On May 15, 2016, 5:03 p.m., Kåre Särs wrote: > > Looks good :) A couple of questions: > > > > - If we create the .rcc do we also want to install the icons? > > - I creates a similar solution for Kate on Windows (in a separate repo), > > but I needed to add a program to replace the symlinked files with aliases > > in the .qrc as symlinks do not work properly on windows. Are you interested > > in adding a similar program/script/... to do the same directly in > > breeze-icons.git? My application is in > > git://anongit.kde.org/scratch/sars/kate-windows.git (icon-rcc) > > Gleb Popov wrote: > > If we create the .rcc do we also want to install the icons? > > No idea, to be honest. > > > I creates a similar solution for Kate on Windows (in a separate repo), > but I needed to add a program to replace the symlinked files with aliases in > the .qrc as symlinks do not work properly on windows > > Sorry, what symlinks? I saw some symlinks stuff in autotests, but didn't > get what it is all about. Quite a lot of icons in breeze-icons are just symbolic links to other icons in the repository. On Linux/OSX/Android/... this is not a problem, but on Windows, where you don't have proper symbolic links, git just creates a text file with the path to the target file. With those text files you just don't get an icon :( So my solution was to create an application to modify the .qrc file add an alias to the target file in stead of using the symbolic link file directly. Don't let this comment be in the way of committing this RR :) - Kåre --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127911/#review95485 --- On May 16, 2016, 6:21 a.m., Gleb Popov wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127911/ > --- > > (Updated May 16, 2016, 6:21 a.m.) > > > Review request for KDE Frameworks. > > > Repository: breeze-icons > > > Description > --- > > I copied icons into the binary dir, because i haven't found a way to generate > rcc without polluting source dir. > > Not sure if installation dir is right, too. > > > Diffs > - > > CMakeLists.txt 2147705 > icons-dark/CMakeLists.txt 36d37f1 > icons/CMakeLists.txt 5ded49c > > Diff: https://git.reviewboard.kde.org/r/127911/diff/ > > > Testing > --- > > > Thanks, > > Gleb Popov > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127911: Add a CMake option to build binary Qt resource out of icons dir.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127911/#review95485 --- Looks good :) A couple of questions: - If we create the .rcc do we also want to install the icons? - I creates a similar solution for Kate on Windows (in a separate repo), but I needed to add a program to replace the symlinked files with aliases in the .qrc as symlinks do not work properly on windows. Are you interested in adding a similar program/script/... to do the same directly in breeze-icons.git? My application is in git://anongit.kde.org/scratch/sars/kate-windows.git (icon-rcc) - Kåre Särs On May 14, 2016, 8:27 a.m., Gleb Popov wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127911/ > --- > > (Updated May 14, 2016, 8:27 a.m.) > > > Review request for KDE Frameworks. > > > Repository: breeze-icons > > > Description > --- > > I copied icons into the binary dir, because i haven't found a way to generate > rcc without polluting source dir. > > Not sure if installation dir is right, too. > > > Diffs > - > > icons/CMakeLists.txt 5ded49c > > Diff: https://git.reviewboard.kde.org/r/127911/diff/ > > > Testing > --- > > > Thanks, > > Gleb Popov > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127896: make dbus optional on osx: kauth
> On May 12, 2016, 6:15 a.m., Kåre Särs wrote: > > I think this patch should not include any platform specific defines. > > Disabling DBus requirement on Windows might also be interesting for some > > projects. I propose to do something similar to what is done in kxmlgui to > > disable kglobalaccel. The default is to require kglobalaccel, but if you > > knowingly specify "-DFORCE_DISABLE_KGLOBALACCEL=1" kglobalaccel is not > > required or searched for. > > René J.V. Bertin wrote: > True, even if there's probably very little point in disabling DBus on a > standard Unix/Linux set-up. > > But indeed, a platform-agnostic option can still be included in the > options that will be flipped by a platform-specific option like > `APPLE_STANDALONE_BUNDLE` I've proposed in a patch for Marble. > > To come back to what I suggested earlier: even if there were to be a KF5 > framework that encapsulates DBus or "something more platform native" there > would still be a use-case for using DBus through that framework even on OS X > (or MS Windows). Projects like HomeBrew, MacPorts and Cygwin don't exist to > replace the native desktop, but to complement it; to provide an easy way to > install and maintain FOSS and provide a context in which those applications > can run as much as possible as they do on their native platform. That > evidently includes DBus, but not just so that Gnome apps can talk with other > Gnome apps and KDE apps with other KDE apps. I don't have any stats on this, > but I'd expect that a good many of the users of such projects use them not so > much for software development per se, but as tools for their actual work > (often in science and/or engineering, in my impression). That might very well > include using Gnome/GTk apps alongside KDE applications, and in that case > they'd proba bly expect the same kind of interoperability as those apps would have on the other platforms they might use. > IOW, I don't think a distribution system that aims to provide the best > possible context for the applications it carries can get around using DBus. > > But this is probably not the best place for an in-depth discussion around > underlying principles; that should probably be done on a ML (and ideally > include a DBus dev or two). > > Nick Shaforostoff wrote: > i don't see a reason behind introducing a special cmake option other than > code coverage (test dbus and dbus-free branches with the same qt): why one > should want to disable dbus on a system where he/she has Qt with dbus? > > can you explain this to me? > > regarding homebrew, i repeat: by default it installs a precompiled > version of Qt without dbus. if user wants dbus then he/she has to have > homebrew recompile whole Qt (takes about 1 hour). and what if kde app doesn't > need any IPC? that would just pollute his/her system with redundant stuff. > how many gtk-based apps require dbus to run on windows and osx? We have Kate as an example. At the moment the main thing we need DBus for on windows is opening documents in only one window when opening new documents through explorer. Without the DBus daemon it does not work. KDevelop has a solution for it without using DBus which means that we could skip DBus usage for this purpose and we would not need to tell people to install the service (DBus) that occasionally has been flagged as mallware. You might argue that we should just compile support without installing the service, but how do I know what features don't work without the service? If the frameworks need special options to disable DBus I'm informed about what is disabled and can choose if I need it or not. Without the option the feature is silently disabled. This is the reason I want separate options for each framework that provides it and not a global one in ECM. Almost all Qt users on Windows that I know of would not even dream of compiling their own Qt and pre-compiled Qt comes with DBus support. - Kåre --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127896/#review95405 --- On May 12, 2016, 5:16 a.m., Nick Shaforostoff wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127896/ > --- > > (Updated May 12, 2016, 5:16 a.m.) > > > Review request for KDE Software on Mac OS X, KDE Frameworks, David Edmundson, > and Martin Gräßlin. > > > Repository: kauth > > > Description &
Re: Review Request 127896: make dbus optional on osx: kauth
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127896/#review95405 --- I think this patch should not include any platform specific defines. Disabling DBus requirement on Windows might also be interesting for some projects. I propose to do something similar to what is done in kxmlgui to disable kglobalaccel. The default is to require kglobalaccel, but if you knowingly specify "-DFORCE_DISABLE_KGLOBALACCEL=1" kglobalaccel is not required or searched for. - Kåre Särs On May 12, 2016, 5:16 a.m., Nick Shaforostoff wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127896/ > --- > > (Updated May 12, 2016, 5:16 a.m.) > > > Review request for KDE Software on Mac OS X, KDE Frameworks, David Edmundson, > and Martin Gräßlin. > > > Repository: kauth > > > Description > --- > > this is the first patch to make kde frameworks build (and then work) without > dbus. > > this will allow homebrew users use precompiled vanilla Qt to build kde apps > on osx. as dbus is not a common service in osx world, kde apps on osx should > use native means for interprocess communication instead -- this will make > them better citizens in osx ecosystem. > > > Diffs > - > > CMakeLists.txt 48dc2d9 > autotests/BackendsManager.cpp 59675b3 > autotests/CMakeLists.txt b53d760 > autotests/HelperTest.cpp 8050a06 > src/CMakeLists.txt 1b6930d > src/ConfigureChecks.cmake d46761a > > Diff: https://git.reviewboard.kde.org/r/127896/diff/ > > > Testing > --- > > compiles fine on osx, compiles fine on linux, tests on linux still pass. > > > Thanks, > > Nick Shaforostoff > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127587: [Kate view] Correct override cursor
> On April 8, 2016, 7:29 a.m., Kåre Särs wrote: > > What is it that needs fixing? > > > > I don't have a problem in either case listed. > > Emmanuel Pescosta wrote: > > 3. Context menu is closed > > I can reproduce this problem > > Anthony Fieroni wrote: > Drag file from Dolphin to Kate. When drag ends (drop or cancel) cursor > stays pointer. Plays Kate on bottom right corner, maximize with double click > on title => cursor stays pointer. Context menu has a stupid Qt bug, if cursor > is over him and hit ESC cursor became caret, but if you cancel context menu > and curson is not over him stays pointer. I try to fix it but setCursor not > works in that way. https://bugreports.qt.io/browse/QTBUG-4514 > > Kåre Särs wrote: > Sorry I can not reproduce the problems even the more detailed > description. This is on Kubuntu 15.10 with neon repos and on Windows. > > What version of Qt and distro are you using and is it Wayland/X11/...? > > David Rosca wrote: > > 3. Context menu is closed > > This is regression in Qt 5.6 https://bugreports.qt.io/browse/QTBUG-49222 > > Anthony Fieroni wrote: > KaOS here, Qt 5.6, kf 5.21 Drag, maximize happens every time. > > Anthony Fieroni wrote: > I can't image you can't reproduce since it has many ways to do it. > https://youtu.be/WYwhVDJKpMo It seems quite clear to me that it is Qt 5.6 that has a regression in the xcb platform plugin. My kubuntu uses Qt 5.5.1 and does not seem to be effected. The patch makes it work, but it is not the "proper" fix. I hope Qt 5.6.1 will fix it, but if not we might have to make a temporary workaround... I feel a bit uncomfortable with adding workarounds that might not be needed in a couple of weeks... - Kåre --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127587/#review94421 --- On April 8, 2016, 4:36 a.m., Anthony Fieroni wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127587/ > --- > > (Updated April 8, 2016, 4:36 a.m.) > > > Review request for Kate, KDE Frameworks, Dominik Haumann, and Kåre Särs. > > > Bugs: 361075 > https://bugs.kde.org/show_bug.cgi?id=361075 > > > Repository: ktexteditor > > > Description > --- > > Currsor must be override after these conditions: > 1. Drag move ends > 2. Minimize / restore / maximize > 3. Context menu is closed > > > Diffs > - > > src/view/kateviewinternal.cpp ca5acfc > > Diff: https://git.reviewboard.kde.org/r/127587/diff/ > > > Testing > --- > > > Thanks, > > Anthony Fieroni > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127587: [Kate view] Correct override cursor
> On April 8, 2016, 7:29 a.m., Kåre Särs wrote: > > What is it that needs fixing? > > > > I don't have a problem in either case listed. > > Emmanuel Pescosta wrote: > > 3. Context menu is closed > > I can reproduce this problem > > Anthony Fieroni wrote: > Drag file from Dolphin to Kate. When drag ends (drop or cancel) cursor > stays pointer. Plays Kate on bottom right corner, maximize with double click > on title => cursor stays pointer. Context menu has a stupid Qt bug, if cursor > is over him and hit ESC cursor became caret, but if you cancel context menu > and curson is not over him stays pointer. I try to fix it but setCursor not > works in that way. https://bugreports.qt.io/browse/QTBUG-4514 Sorry I can not reproduce the problems even the more detailed description. This is on Kubuntu 15.10 with neon repos and on Windows. What version of Qt and distro are you using and is it Wayland/X11/...? - Kåre --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127587/#review94421 --- On April 8, 2016, 4:36 a.m., Anthony Fieroni wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127587/ > --- > > (Updated April 8, 2016, 4:36 a.m.) > > > Review request for Kate, KDE Frameworks, Dominik Haumann, and Kåre Särs. > > > Bugs: 361075 > https://bugs.kde.org/show_bug.cgi?id=361075 > > > Repository: ktexteditor > > > Description > --- > > Currsor must be override after these conditions: > 1. Drag move ends > 2. Minimize / restore / maximize > 3. Context menu is closed > > > Diffs > - > > src/view/kateviewinternal.cpp ca5acfc > > Diff: https://git.reviewboard.kde.org/r/127587/diff/ > > > Testing > --- > > > Thanks, > > Anthony Fieroni > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127587: [Kate view] Correct override cursor
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127587/#review94421 --- What is it that needs fixing? I don't have a problem in either case listed. - Kåre Särs On April 8, 2016, 4:36 a.m., Anthony Fieroni wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127587/ > --- > > (Updated April 8, 2016, 4:36 a.m.) > > > Review request for Kate, KDE Frameworks, Dominik Haumann, and Kåre Särs. > > > Bugs: 361075 > https://bugs.kde.org/show_bug.cgi?id=361075 > > > Repository: ktexteditor > > > Description > --- > > Currsor must be override after these conditions: > 1. Drag move ends > 2. Minimize / restore / maximize > 3. Context menu is closed > > > Diffs > - > > src/view/kateviewinternal.cpp ca5acfc > > Diff: https://git.reviewboard.kde.org/r/127587/diff/ > > > Testing > --- > > > Thanks, > > Anthony Fieroni > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126618: Make CMake macro kcoreaddons_desktop_to_json() work on windows.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126618/ --- (Updated March 22, 2016, 4:53 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Changes --- Submitted with commit 7ac7a605923f0bb2f0f367f6069a101a24bead9f by Kåre Särs to branch master. Repository: kcoreaddons Description --- Without this patch kcoreaddons_desktop_to_json() will not find the service type destop file. On windows GenericDataLocation returns "C:/Users//AppData/Local" or "C:/ProgramData". That is not a path that contains any destop files ;) This patch adds KDE_INSTALL_FULL_KSERVICETYPES5DIR as fist item in the search paths and if not found searches like before. Diffs - KF5CoreAddonsMacros.cmake 5d8e3d4 Diff: https://git.reviewboard.kde.org/r/126618/diff/ Testing --- KTextEditor compiles on windows Thanks, Kåre Särs ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127237: Fix crash in kmore tools on Windows
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127237/ --- (Updated March 11, 2016, 12:43 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Gregor Mi. Changes --- Submitted with commit def01c5bb668a284d9db48c4c402adc8268b895a by Kåre Särs to branch master. Repository: knewstuff Description --- On Windows we sometimes get null-pointers in stead of pointers to KMoreToolsService. This patch adds checks for null-pointers in those places that made Kate crash in the project plugin and adds a nullptr check before adding them to the list. Diffs - src/kmoretools/kmoretoolsmenufactory.cpp 30f4d02 src/kmoretools/kmoretoolspresets.cpp 2405321 Diff: https://git.reviewboard.kde.org/r/127237/diff/ Testing --- Compiled and run on windows. No crashes in Kate when right-clicking files in the project plugin. Thanks, Kåre Särs ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126618: Make CMake macro kcoreaddons_desktop_to_json() work on windows.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126618/ --- (Updated March 11, 2016, 8:19 p.m.) Review request for KDE Frameworks. Changes --- Changed to FULL version of KSERVICETYPES5DIR and remove the unneeded additional search path. Repository: kcoreaddons Description (updated) --- Without this patch kcoreaddons_desktop_to_json() will not find the service type destop file. On windows GenericDataLocation returns "C:/Users//AppData/Local" or "C:/ProgramData". That is not a path that contains any destop files ;) This patch adds KDE_INSTALL_FULL_KSERVICETYPES5DIR as fist item in the search paths and if not found searches like before. Diffs (updated) - KF5CoreAddonsMacros.cmake 5d8e3d4 Diff: https://git.reviewboard.kde.org/r/126618/diff/ Testing --- KTextEditor compiles on windows Thanks, Kåre Särs ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127237: Fix crash in kmore tools on Windows
> On March 7, 2016, 9:29 p.m., Gregor Mi wrote: > > src/kmoretools/kmoretoolsmenufactory.cpp, lines 128-129 > > <https://git.reviewboard.kde.org/r/127237/diff/2/?file=448481#file448481line128> > > > > Thanks for the comment. However, as long as we don't know the root > > cause for the null pointers, I would feel better if the comment states > > clearly that we don't know what is happening and that it is happening on > > Windows only. > > Kåre Särs wrote: > This one is not needed at the moment (I did not get null pointers here), > but I left it there just in case. I feel it is better to loose functionality > than getting a crash ;) > > Gregor Mi wrote: > Then it's ok. General question: is there a common log file where one > could write a message in case there is nullptr which was catched? .xsession-errors/stderr probably get the qCritical() debug output and dbgviewer on windows is logging it if running. - Kåre --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127237/#review93270 --- On March 7, 2016, 8:38 p.m., Kåre Särs wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127237/ > --- > > (Updated March 7, 2016, 8:38 p.m.) > > > Review request for KDE Frameworks and Gregor Mi. > > > Repository: knewstuff > > > Description > --- > > On Windows we sometimes get null-pointers in stead of pointers to > KMoreToolsService. This patch adds checks for null-pointers in those places > that made Kate crash in the project plugin and adds a nullptr check before > adding them to the list. > > > Diffs > - > > src/kmoretools/kmoretoolsmenufactory.cpp 30f4d02 > src/kmoretools/kmoretoolspresets.cpp 2405321 > > Diff: https://git.reviewboard.kde.org/r/127237/diff/ > > > Testing > --- > > Compiled and run on windows. No crashes in Kate when right-clicking files in > the project plugin. > > > Thanks, > > Kåre Särs > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126618: Make CMake macro kcoreaddons_desktop_to_json() work on windows.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126618/ --- (Updated March 8, 2016, 8:35 p.m.) Review request for KDE Frameworks. Changes --- Add KDE_INSTALL_KSERVICETYPES5DIR as the first search path (in CMake) Repository: kcoreaddons Description (updated) --- Without this patch kcoreaddons_desktop_to_json() will not find the service type destop file. On windows GenericDataLocation returns "C:/Users//AppData/Local" or "C:/ProgramData". That is not a path that contains any destop files ;) This patch adds KDE_INSTALL_KSERVICETYPES5DIR as fist item in the search paths and if not found searches like before and adds ../share/ if the previous searches do not return a match. Diffs (updated) - KF5CoreAddonsMacros.cmake 5d8e3d4 src/lib/plugin/desktopfileparser.cpp 319d29f Diff: https://git.reviewboard.kde.org/r/126618/diff/ Testing (updated) --- KTextEditor compiles on windows Thanks, Kåre Särs ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127237: Fix crash in kmore tools on Windows
> On March 7, 2016, 9:29 p.m., Gregor Mi wrote: > > src/kmoretools/kmoretoolsmenufactory.cpp, lines 128-129 > > <https://git.reviewboard.kde.org/r/127237/diff/2/?file=448481#file448481line128> > > > > Thanks for the comment. However, as long as we don't know the root > > cause for the null pointers, I would feel better if the comment states > > clearly that we don't know what is happening and that it is happening on > > Windows only. This one is not needed at the moment (I did not get null pointers here), but I left it there just in case. I feel it is better to loose functionality than getting a crash ;) > On March 7, 2016, 9:29 p.m., Gregor Mi wrote: > > src/kmoretools/kmoretoolsmenufactory.cpp, lines 237-238 > > <https://git.reviewboard.kde.org/r/127237/diff/2/?file=448481#file448481line237> > > > > see my comment above same as above ;) > On March 7, 2016, 9:29 p.m., Gregor Mi wrote: > > src/kmoretools/kmoretoolspresets.cpp, line 97 > > <https://git.reviewboard.kde.org/r/127237/diff/2/?file=448482#file448482line97> > > > > Again, this should not happen here, so please add a comment that this > > is a fix for Windows. This is the first place where we get the null pointers. KMoreTools::registerServiceByDesktopEntryName() has at least two code paths that return nullptr. kmoretools.cpp line 129: qCritical() << ... the kmt-desktopfile " << desktopEntryName << " is provided but no Exec line is specified ... line 130: return nullptr; and line 146: qCritical() << ... a kmt-desktopfile must be provided. Please fix. Return nullptr. ... line 147: return nullptr; These might be errors, but I would rather see that it does not crash even if a runtime file is missing. I suspect that it is the later that generates the nullptr. This is why I check the returned pointer before using it. QStandardPaths::GenericDataLocation does not return ../share/ on Windows and thus does not find the file > On March 7, 2016, 9:29 p.m., Gregor Mi wrote: > > src/kmoretools/kmoretoolspresets.cpp, lines 165-167 > > <https://git.reviewboard.kde.org/r/127237/diff/2/?file=448482#file448482line165> > > > > As above: as long as we don't know the reason for the nullpointers I > > would be happier if there was a comment. > > > > That said I don't know what the KDE policy is when dealing with this > > kind of problem. Just adding null checks seems to make the code more > > complicated to analyse later. You are a long-term committer, so your words > > have a high weight. On the other hand I would be interested in a second > > opinion. KMoreToolsPresets::registerServiceByDesktopEntryName() has one code path that returns a direct nullptr and another that calls KMoreTools::registerServiceByDesktopEntryName() that also can return a nullptr. So I cannot be sure that the pointer isn't null. So check it and don't add nullptr ;) The problem is, with 99% certainty, the missing .desktop file, but IMO we should not crash even if a runtime file is missing ;) - Kåre --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127237/#review93270 --- On March 7, 2016, 8:38 p.m., Kåre Särs wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127237/ > --- > > (Updated March 7, 2016, 8:38 p.m.) > > > Review request for KDE Frameworks and Gregor Mi. > > > Repository: knewstuff > > > Description > --- > > On Windows we sometimes get null-pointers in stead of pointers to > KMoreToolsService. This patch adds checks for null-pointers in those places > that made Kate crash in the project plugin and adds a nullptr check before > adding them to the list. > > > Diffs > - > > src/kmoretools/kmoretoolsmenufactory.cpp 30f4d02 > src/kmoretools/kmoretoolspresets.cpp 2405321 > > Diff: https://git.reviewboard.kde.org/r/127237/diff/ > > > Testing > --- > > Compiled and run on windows. No crashes in Kate when right-clicking files in > the project plugin. > > > Thanks, > > Kåre Särs > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127237: Fix crash in kmore tools on Windows
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127237/ --- (Updated March 7, 2016, 8:38 p.m.) Review request for KDE Frameworks and Gregor Mi. Changes --- It is not related to DBus (Just Windows ;) Added comments and a nullptr check before adding a the pointer to the kmtServiceList Summary (updated) - Fix crash in kmore tools on Windows Repository: knewstuff Description (updated) --- On Windows we sometimes get null-pointers in stead of pointers to KMoreToolsService. This patch adds checks for null-pointers in those places that made Kate crash in the project plugin and adds a nullptr check before adding them to the list. Diffs (updated) - src/kmoretools/kmoretoolsmenufactory.cpp 30f4d02 src/kmoretools/kmoretoolspresets.cpp 2405321 Diff: https://git.reviewboard.kde.org/r/127237/diff/ Testing (updated) --- Compiled and run on windows. No crashes in Kate when right-clicking files in the project plugin. Thanks, Kåre Särs ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127265: Fix windows build of Ki18n
> On March 3, 2016, 4:51 p.m., Aleix Pol Gonzalez wrote: > > src/kcatalog.cpp, line 150 > > <https://git.reviewboard.kde.org/r/127265/diff/1/?file=447929#file447929line150> > > > > Wouldn't it be the same to define it like this? > > ``` > > #if defined(__USE_GNU_GETTEXT) > > extern "C" int Q_DECL_IMPORT _nl_msg_cat_cntr; > > #endif > > ``` > > > > This way we wouldn't have to make special cases for each platform. > > > > I just tried and works here (on Linux+Clang). > > Andre Heinecke wrote: > Ah nice, did not know / remembered this macro. I think in that case we > can also drop the extern int declaration inside the function as we already > declare it now in a platform independent manner as an external variable. > > I've tested this again with mingw and GNU/Linux GCC and it still compiled. > > Andreas Cord-Landwehr wrote: > Just tested, also works fine under Android. +1 I think the bug in the last review change was that the extern declaration was inside the function. I'm not sure if the #ifdef was a real problem as you probably need the __declspec() also with MinGW. This solution is however the best so far :) - Kåre --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127265/#review93104 --- On March 3, 2016, 5:49 p.m., Andre Heinecke wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127265/ > ------- > > (Updated March 3, 2016, 5:49 p.m.) > > > Review request for KDE Frameworks, Andreas Cord-Landwehr and Kåre Särs. > > > Repository: ki18n > > > Description > --- > > In review Request 127048 (Restrict _nl_msg_cat_cntr use to GNU gettext > implentations.) Unrelated Windows changes were requested and then introduced > which in my opinion were wrong (and in the opinion of my compiler ;-) ). > > You can't just do an extern "C" dllimport declaration inside a function body: > > src/kcatalog.cpp:190:16: error: expected unqualified-id before string constant > extern "C" int __declspec(dllimport) _nl_msg_cat_cntr; > > > Also that patch changed the Logic from an MSVC specific define to general > Q_OS_WIN. This appears to be wrong, too. While I'm only testing with MinGW > I'm pretty sure that the ifndef _MSC_VER was there for a reason. > > This patch reverts the declaration move and logic change while keeping the > __USE_GNU_GETTEXT guard. > > > Diffs > - > > src/kcatalog.cpp 083443d > > Diff: https://git.reviewboard.kde.org/r/127265/diff/ > > > Testing > --- > > Compiled with mingw and __USE_GNU_GETTEXT for Windows. Also compiled for > GNU/Linux. > > > Thanks, > > Andre Heinecke > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 127237: Fix crash in kmore tools when DBus is missing
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127237/ --- Review request for KDE Frameworks and Gregor Mi. Repository: knewstuff Description --- If DBus is missing we get null-pointers in stead of KMoreToolsService*. This patch adds checks for null-pointers in those places that made Kate crash in the project plugin. Diffs - src/kmoretools/kmoretoolsmenufactory.cpp 30f4d02 src/kmoretools/kmoretoolspresets.cpp 2405321 Diff: https://git.reviewboard.kde.org/r/127237/diff/ Testing --- Compiled and run on windows. No crashes in Kate when running without DBus and right-clicking files in the project plugin. Thanks, Kåre Särs ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127033: Add /../share/hunspell/ to dictionary search path
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127033/ --- (Updated Feb. 24, 2016, 8:25 p.m.) Review request for KDE Frameworks, Laurent Montel and Martin Tobias Holmedahl Sandsmark. Changes --- Use QFile::encodeName() and add '/' in composeDictName() Repository: sonnet Description --- The default search path for hunspell dictionaries is /usr/share/hunspell/, but that is not a very common directory on windows ;) For Windows and Mac there are some extra search paths defined in side #ifdefs, but they are hardcoded to the LibreOffice dictionary directories. Unfortunately the extra Windows path points to a 32bit LibreOffice 5 on a 64 bit Windows directory. This patch adds /../share/hunspell/ to the search path in case the default is not found. If wanted I can add a CMake define for specifying the path relative to the application directory. Diffs (updated) - src/core/guesslanguage.cpp e8deb90 src/plugins/hunspell/CMakeLists.txt 144515b src/plugins/hunspell/hunspellclient.cpp 3da7219 src/plugins/hunspell/hunspelldict.h b825d7d src/plugins/hunspell/hunspelldict.cpp bc5c314 Diff: https://git.reviewboard.kde.org/r/127033/diff/ Testing --- Compiled Sonnet and Kate on Windows and got spelling to work :) It is now possible to add dictionaries to a relocatable /../share/hunspell/ and get spelling to work without depending on LibreOffice installation on Windows. Thanks, Kåre Särs ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127033: Add /../share/hunspell/ to dictionary search path
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127033/ --- (Updated Feb. 24, 2016, 8:25 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks, Laurent Montel and Martin Tobias Holmedahl Sandsmark. Changes --- Submitted with commit af4c6011d545fec7bf5a1a0a548ae68aa96e2fc8 by Kåre Särs to branch master. Repository: sonnet Description --- The default search path for hunspell dictionaries is /usr/share/hunspell/, but that is not a very common directory on windows ;) For Windows and Mac there are some extra search paths defined in side #ifdefs, but they are hardcoded to the LibreOffice dictionary directories. Unfortunately the extra Windows path points to a 32bit LibreOffice 5 on a 64 bit Windows directory. This patch adds /../share/hunspell/ to the search path in case the default is not found. If wanted I can add a CMake define for specifying the path relative to the application directory. Diffs - src/core/guesslanguage.cpp e8deb90 src/plugins/hunspell/CMakeLists.txt 144515b src/plugins/hunspell/hunspellclient.cpp 3da7219 src/plugins/hunspell/hunspelldict.h b825d7d src/plugins/hunspell/hunspelldict.cpp bc5c314 Diff: https://git.reviewboard.kde.org/r/127033/diff/ Testing --- Compiled Sonnet and Kate on Windows and got spelling to work :) It is now possible to add dictionaries to a relocatable /../share/hunspell/ and get spelling to work without depending on LibreOffice installation on Windows. Thanks, Kåre Särs ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127033: Add /../share/hunspell/ to dictionary search path
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127033/ --- (Updated Feb. 22, 2016, 8:59 p.m.) Review request for KDE Frameworks, Laurent Montel and Martin Tobias Holmedahl Sandsmark. Changes --- Add a static function to compose the dictionary name from the path and language Use the static function Replace QFileInfo(dic).exists() with QFileInfo::exists(dic) Repository: sonnet Description --- The default search path for hunspell dictionaries is /usr/share/hunspell/, but that is not a very common directory on windows ;) For Windows and Mac there are some extra search paths defined in side #ifdefs, but they are hardcoded to the LibreOffice dictionary directories. Unfortunately the extra Windows path points to a 32bit LibreOffice 5 on a 64 bit Windows directory. This patch adds /../share/hunspell/ to the search path in case the default is not found. If wanted I can add a CMake define for specifying the path relative to the application directory. Diffs (updated) - src/core/guesslanguage.cpp e8deb90 src/plugins/hunspell/hunspellclient.cpp 3da7219 src/plugins/hunspell/hunspelldict.cpp bc5c314 Diff: https://git.reviewboard.kde.org/r/127033/diff/ Testing --- Compiled Sonnet and Kate on Windows and got spelling to work :) It is now possible to add dictionaries to a relocatable /../share/hunspell/ and get spelling to work without depending on LibreOffice installation on Windows. Thanks, Kåre Särs ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: RCC for icons - update: Re: Icons installed by apps
On Thursday, February 18, 2016 02:51:58 PM René J.V. Bertin wrote: > On Thursday February 18 2016 13:55:18 Jaroslaw Staniek wrote: > >>> > since QIcon::fromTheme() apaprently isn't able to find app icons. > > Care to explain? QIcon::fromTheme() doesn't find anything "out of the box" > on OS X (and I presume on MS Windows), but that is only because no theme > search path is set on those platforms. When you add the standard XDG icon > repository to the icon search path on OS X, even "pure" Qt5 application > will start showing icons all over if you have an icon theme installed -- > including in widgets that should not have icons according to the HIG. Also > on OS X, fromTheme() will only return the application icon (as in the icon > shown in the Finder) if the current theme defines that same icon for the > calling application, and the theme search path is set of course. In all > other cases it will not, because the application icon is not defined > through a theme on OS X (nor is it on MS Windows, I presume). > >> I think the solution with a packaged breeze icons resource working > >> out-of-the box could be a good (lightweight) addition for non-Plasma > >> (non-Linux?) users of KF5, to popularize KF5. They grab the icons package > > Popularise, with Breeze "art"work? O:-) > Anyway, I don't think "grabbing an icon package" will work on OS X, not if > you want to create standalone app bundles which by definition contain > everything they need. > >> and icons just work without thousands of files, caching, etc. 'One in a > >> million' would of these users would be interested in theming. > > I'd up that estimate if we're still talking about Breeze icons here O:-) > > >> PS2: I have been beaten by situations such as KToolBar setting 0-size > >> icons by default. > > Partly this is because almost no KF5 code uses the fallback argument of > QIcon::fromTheme() explicitly, which means that the function returns an > empty icon if the search fails. In particular, statements like > > app->setWindowIcon(QIcon::fromTheme(programName)) > > should read > > app->setWindowIcon(QIcon::fromTheme(programName, app->windowIcon())) > > >> This is in a Windows env where no themes are (properly) installed. If the > >> rcc-based solution is in use I would imagine that ideally all the KF5 > >> code > >> detects this somehow and would not look for xdg standard themes through > >> the > >> classic KIconLoader but silently adapt so the rcc resource works great. > >> Just a dream. > > If your rcc resource corresponds to the resource mentioned in the > QIcon::fromTheme() documentation (I think that talks about "qrc") and if I > interpret that documentation correctly then yes, code using that function > will find icons from the rcc/qrc "builtin" resource over those in xdg > themes (if the XDG icon repository is even in the icon theme search path). > >>> What I don't know however is whether artists consider that these icons > >>> should be themeable... > > I think icon artists will consider that you should touch their icons (for > theming or anything else). They will probably also consider that their > icons are the "best" but they really should also consider it a right for > anyone to use other icons ;) > The breeze.rcc file way is actually how Christoph solved it in Kate for Windows and OSX. We create an .rcc file from the breeze icons and at start-up we search for the file in QStandardPaths::DataLocation and if the file is found we load it with registerResource() and add ":/icons" to the themeSearchPaths. (kate.git/icons.h) Christoph also added ":/icons" to the search path in kicontheme so that xmlgui toolbars and friends also get the icons from the .rcc file. We get all the same icons as on Linux neatly in one file :) breesze's symlinks was a problem on windows, but I created a small application to replace the symlinked files with an alias pointing to the target file -> smaller .rcc file :) /Kåre ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126876: Fix QFileDialog::openUrl() for remote files
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126876/ --- (Updated Feb. 15, 2016, 7:45 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks, Alex Richardson and David Faure. Changes --- Submitted with commit 5420b5752a30c294486fbaf967187c8a6f64fe42 by Kåre Särs to branch master. Repository: frameworkintegration Description --- Qt does not know if the remote URL points to a file or directory. that is why options()->initialDirectory() returns the full URL even if it is a file. This fix is a bit like Alex Richardson workaround in KIO (https://git.reviewboard.kde.org/r/126831/), but in frameworkintegration in stead (I did not see his/Your KIO fix before now...) I check the remote url in setDirectory() because setDirectoy() is called from two places. Diffs - src/platformtheme/kdeplatformfiledialoghelper.cpp 11e7efb Diff: https://git.reviewboard.kde.org/r/126876/diff/ Testing --- Kate now happily opens local and remote folders :) File Attachments firefox.desktop https://git.reviewboard.kde.org/media/uploaded/files/2016/01/25/60c21962-396e-468e-add9-e7112c49d7ba__firefox.desktop Thanks, Kåre Särs ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126876: Fix QFileDialog::openUrl() for remote files
> On Feb. 14, 2016, 11:11 p.m., Aleix Pol Gonzalez wrote: > > src/platformtheme/kdeplatformfiledialoghelper.cpp, line 192 > > <https://git.reviewboard.kde.org/r/126876/diff/2/?file=444275#file444275line192> > > > > Wouldn't it be a good practice to assume people are doing the right > > thing and not passing something that isn't a folder? > > > > Do other implementations make this check? I doubt it. >From the QFileDialog manual: getOpenFileUrls(): "The function is used similarly to QFileDialog::getOpenFileNames(). In particular parent, caption, dir, filter, selectedFilter and options are used in the exact same way." And in getOpenFileNames(): "The file dialog's working directory will be set to dir. If dir includes a file name, the file will be selected." Unfortunately this does not work for remote URLs as Qt can't know if it points to a file or directory. That is why we need to check it here. - Kåre --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126876/#review92368 --- On Feb. 14, 2016, 6:25 a.m., Kåre Särs wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126876/ > --- > > (Updated Feb. 14, 2016, 6:25 a.m.) > > > Review request for KDE Frameworks, Alex Richardson and David Faure. > > > Repository: frameworkintegration > > > Description > --- > > Qt does not know if the remote URL points to a file or directory. that is why > options()->initialDirectory() returns the full URL even if it is a file. > > > This fix is a bit like Alex Richardson workaround in KIO > (https://git.reviewboard.kde.org/r/126831/), but in frameworkintegration in > stead (I did not see his/Your KIO fix before now...) > > I check the remote url in setDirectory() because setDirectoy() is called from > two places. > > > Diffs > - > > src/platformtheme/kdeplatformfiledialoghelper.cpp 11e7efb > > Diff: https://git.reviewboard.kde.org/r/126876/diff/ > > > Testing > --- > > Kate now happily opens local and remote folders :) > > > File Attachments > > > firefox.desktop > > https://git.reviewboard.kde.org/media/uploaded/files/2016/01/25/60c21962-396e-468e-add9-e7112c49d7ba__firefox.desktop > > > Thanks, > > Kåre Särs > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126876: Fix QFileDialog::openUrl() for remote files
> On Feb. 14, 2016, 11:12 p.m., Aleix Pol Gonzalez wrote:
> > src/platformtheme/kdeplatformfiledialoghelper.cpp, line 198
> > <https://git.reviewboard.kde.org/r/126876/diff/2/?file=444275#file444275line198>
> >
> > We shouldn't be calling sync API there.
That is true :(
I have been debugging this a bit and it is not very straight forward... When we
open the QFileDialog we actually get 3 calls to setDirectory() I have been
thinking about figuring out why it is called so many times but I have not been
able to pinpoint what would be needed to get it to only set the directory and
file once.
KDirSelectDialog is using job->exec() in 4 places and the
KDirOperator::setUrl() would also use job->exec()
but maybe that is the place it should be fixed?
in KDirOperator::setUrl() (kio/src/filewidgets/kdiroperator.cpp line 1005) we
have:
if (!Private::isReadable(newurl)) {
// maybe newurl is a file? check its parent directory
And here we go on to use job->exec() and friends to check if the URL is
readable and a directory. Except that Private::isReadable() always returns true
for remote URLs...
What is the proper solution here?
- Kåre
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126876/#review92369
---
On Feb. 14, 2016, 6:25 a.m., Kåre Särs wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126876/
> ---
>
> (Updated Feb. 14, 2016, 6:25 a.m.)
>
>
> Review request for KDE Frameworks, Alex Richardson and David Faure.
>
>
> Repository: frameworkintegration
>
>
> Description
> ---
>
> Qt does not know if the remote URL points to a file or directory. that is why
> options()->initialDirectory() returns the full URL even if it is a file.
>
>
> This fix is a bit like Alex Richardson workaround in KIO
> (https://git.reviewboard.kde.org/r/126831/), but in frameworkintegration in
> stead (I did not see his/Your KIO fix before now...)
>
> I check the remote url in setDirectory() because setDirectoy() is called from
> two places.
>
>
> Diffs
> -
>
> src/platformtheme/kdeplatformfiledialoghelper.cpp 11e7efb
>
> Diff: https://git.reviewboard.kde.org/r/126876/diff/
>
>
> Testing
> ---
>
> Kate now happily opens local and remote folders :)
>
>
> File Attachments
> ----
>
> firefox.desktop
>
> https://git.reviewboard.kde.org/media/uploaded/files/2016/01/25/60c21962-396e-468e-add9-e7112c49d7ba__firefox.desktop
>
>
> Thanks,
>
> Kåre Särs
>
>
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
