Re: Incubation Request: Kiview

[Sven Brauch]

Hi,

On 28.04.24 12:22, Méven wrote:

Here is an email for incubating a new KDE project: Kiview.
Kiview is a simple file previewer written in C++/Qml, from its README:


looking through the code there are unfortunately a lot of things one 
trips over.


First and foremost, as others have mentioned, there is way too much 
system() / shell stuff going on. This kind of code tends to be hard to 
get working reliably and easily creates security vulnerabilities.


Instead, I would approach the whole problem differently, by trying to 
leverage the tools KDE Frameworks already offers and uses. Why do you 
need to execute /usr/bin/unzip when KArchive offers this functionality? 
Why use WebEngine when there are already solutions in KDE for rendering 
PDF files? All this could be done in-process without involving 
filesystem operations or shell commands (and probably in a fraction of 
the computation time).


Do you know about the KParts interface? Have a look at that. I think 
it's pretty close to what you want already. E.g. Ark already has a very 
simple document previewer based on that.


Architectural concerns aside, here is a short list of things I noticed 
which should be improved:


 - Don't inherit QThread. Instead, use a more modern task interface 
such as QtConcurrent::run() or std::launch.


 - You do not need threads to asynchronously handle a QProcess. It has 
signals for that.


 - Numbers which have a fixed meaning should be an enum{}.

 - Don't write to files with a fixed name in /tmp. Multiple instances 
of your program will clash and you might overwrite user data. Tools like 
QTemporaryDir exist which avoid these problems.


 - What is going on with the clipboard stuff in dolphinbridge.cpp...? 
That looks like an abuse of the clipboard.


 - Prepending magic prefixes like !isDir! to paths isn't a proper 
solution for anything.


Hope this helps with the next iteration of this program.

Greetings,
Sven


OpenPGP_0xA4AAD0019BE03F15.asc
Description: OpenPGP public key


OpenPGP_signature.asc
Description: OpenPGP digital signature

Re: Should we stop distributing source tarballs?

[Sven Brauch]

Hi,

On 06.04.24 13:07, Marc Deop i Argemí wrote:

If you automate things, everything can be reviewed/validated by more than one
entity and thus increasing security.

The CI can be reviewed and audited but your personal laptop and your workflow
cannot.


This is basically a discussion about whether it is less risky to trust 
the individual developers, or the people with access to the CI signing 
key. You are trading likeliness of there being one bad actor vs. impact 
one bad actor can have. It's a matter of personal opinion; there is no 
right or wrong choice here.


Whenever one option goes wrong, it will be easy to argue for changing to 
the other, until that one goes wrong, at which point you can change back. ;)


IMO the only actual improvement here would be reproducible tarballing: 
if each run of the packaging script produces the same result on all 
systems, the maintainers can locally build the tarball, sign the hash, 
upload the signature, then have the CI system build the same tarball and 
sign it again. Then KDE publishes both signatures and downstreams check 
them both.


I don't know how hard that would be to achieve technically, several 
obstacles come to mind immediately. But it would actually increase trust 
instead of just moving it around.


Greetings,
Sven


OpenPGP_0xA4AAD0019BE03F15.asc
Description: OpenPGP public key


OpenPGP_signature.asc
Description: OpenPGP digital signature

Re: New releases for bugfixes

[Sven Brauch]

Hi,

I think all 3 of us envision very similar things, we just have different 
things we think/talk about, and different understandings of Nate's 
suggestion. I for example understood that Nate suggests to make bugs 
matching the named criteria the *trigger* for making (or discussing) a 
new release. I think you understood it differently, i.e. the maintainers 
initiate this discussion.


On 9/8/22 12:25, Harald Sitter wrote:
> The way I understand the maintainer would do this?

I'd also imagine pretty much this to happen, yes. But as you say, what 
actually triggers the release discussion is "you ask the release team to 
spin an emergency release". To me this is the decision which matters, 
made by the maintainer(s), and everything else is just paperwork to back 
that up.


> Just to be clear, I'm not sure we need the paperwork of having a bug
> and setting it vhi, but we probably do need some workflow to hold on
> to.

Yes, agreed. IMO requiring a bug with certain flags set isn't very 
helpful. I'd rather suggest to go for something like "Out-of-schedule 
bugfix releases are only considered to fix bugs which severely impact an 
application's functionality for one of its core use cases" or the like, 
and then one can argue about whether this definition is met.


At which point the whole thing has nothing to do with bug tickets any 
more, which I understood was the core point Francis was also trying to make.


Greetings,
Sven


OpenPGP_0xA4AAD0019BE03F15.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

Re: New releases for bugfixes

[Sven Brauch]

Hi,

On 9/7/22 17:28, Harald Sitter wrote:

On Wed, Sep 7, 2022 at 5:20 PM  wrote:

In most projects the maintainers who'd make a release decision are the
same people who triage bugs


You quite clearly have no idea how this community works. I'll thank
you not to misdirect discussions.


in kate it also works very much like this, so while "most projects" is a 
bit of a stretch, there are certainly relevant projects where this is 
the case, so Francis' point is well worth discussing.


In fact I share Francis' concern, and I think re-using the 
severity/priority fields to make this decision will add more confusion 
than it provides value. Also consider that a lot of people will set 
these fields in the tracker which are not aware of this policy. I'd 
instead leave the decision to the project maintainers and they can voice 
it by writing an email to some list.


It would be nice if you could address the point if you disagree, instead 
of brushing it off without explanation.


Greetings,
Sven



OpenPGP_0xA4AAD0019BE03F15.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

Re: Approval request for feature idea

[Sven Brauch]

Hi,

On 6/1/22 20:41, samuel ammonius wrote:
However, I still don't see the point of avoiding QSS because it seems to 
be able to do everything CSS can (besides transformations, which are the 
only difference that I've been able to find so far).


Sorry but then you're not looking very hard. Look at e.g. [1].

Just from a quick scroll-through, I find a lot of stuff QSS has never 
heard about, such as animations, box-shadow, caret-color, clipping, 
filter, float, advanced font options, text transform, text shadow, media 
queries, blend mode, overflow, perspective, transitions, n-th-child 
selectors, in general half the selectors, all CSS functions, 
before/after content, etc etc etc.


But that's not even the problem. The problem is that QSS is not a style 
by itself, it is applied *on top of* a style such as Fusion, and does 
*not* give you full control over that style.


So what do you even want to achieve?

Do you want a fully customizable style? QSS isn't, it's not even *a* 
style to begin with.


Do you want to apply some customization while preserving the base looks 
of whatever style the user has configured? Then QSS is a nice thing but 
unless you limit yourself to really basic stuff (mainly colors) some 
widgets will look weird or broken in some base styles, or in some 
applications. They might even break with colors alone, simply from the 
fact that a style sheet is set at all.


I suggest we stop discussing this here at this point, I don't think it's 
very productive. I'd recommend you try to make a complete style changing 
appearance of all widgets (especially the more funky stuff: scrollbars, 
checkable combo boxes, progress bars, tool buttons with dropdowns, 
checkable menu items with icons, tree view items, ...) as you want them 
to look like with QSS, and open a few complicated applications (krita, 
dolphin, kdevelop, gwenview, the KDE file dialogs) with that style. I 
recommend a dark style, it tends to make problems more obvious. Try to 
make it perfect, like you'd actually want it to look like, not a 
prototype. I hope this experience helps you understand the concerns 
raised here. And if not -- well, maybe people here are wrong and this 
idea will fly after all ;)


Greetings,
Sven

_
[1] https://www.w3schools.com/cssref/default.asp


OpenPGP_0xA4AAD0019BE03F15.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

Re: Fwd: Approval request for feature idea

[Sven Brauch]

Hi,

On 5/30/22 20:37, samuel ammonius wrote:
I've worked with regular CSS and I'm sure that stylesheets offer just as 
many customization options as things like QtCurve or QStylePlugins. The 
reason that it may not seem this way is because Qt didn't document 
regular CSS syntax in the documentation for stylesheets.


No, the reason is that Qt's CSS has absolutely nothing to do with the 
regular CSS known from browsers. It's a proxy style which tweaks how a 
base style (e.g. Fusion or Breeze) draws elements on the screen, by e.g. 
modifying the palette and then forwarding the draw call to the base 
style. It implements maybe 1% of the CSS stuff a modern browser can do, 
and that's a favourable estimate. And not even all of that will work as 
expected in all cases. For details, see [1].


That this proxy style's behaviour can be configured using a CSS-like 
syntax is coincidental, or, well, intentionally made that way for ease 
of use. But: this is not the CSS you know or expect.


I can't verify that stylesheets can do everything that a style plugin 
can do


They can't. Regular CSS 3 in Firefox is probably pretty close, 
practically speaking, but Qt's CSS, not so much.


but I know for sure that Breeze can be made using a qstylesheet 


Where did you get this information? This is certainly not the case. Just 
try to make e.g. the animated fades in Breeze using Qt's CSS and Fusion 
as the base style and you will immediately discover that it's not possible.


Greetings,
Sven

_
[1] 
https://code.woboq.org/qt5/qtbase/src/widgets/styles/qstylesheetstyle.cpp.html


OpenPGP_0xA4AAD0019BE03F15.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

Re: Fwd: Approval request for feature idea

[Sven Brauch]

Hi,

On 5/30/22 19:52, samuel ammonius wrote:
Adding this feature won't make the C++ styles disappear. It will only 
make it possible for users who don't know how to make a style plugin in 
C++ to make their own styles


the problem is that the Qt stylesheets are pretty bad at that. The 
customization options they provide are limited, work in unexpected ways, 
and sometimes look flat-out buggy, especially if applications have their 
own widgets or drawing code or if the stylesheet is applied on top of an 
unusual base style.


They are fine for coloring a combo box or line edit in red if the input 
is invalid, but they are not a useful user-configurable theme engine.


I do see the value in the feature you envision, but IMO it needs to 
happen in the form of something like the QtCurve style, which does its 
painting in a way that is directly intended to be customized. 
Customization needs to be offered by the style; it cannot be kludged on 
top of the style with the QCssStyle proxy style, at least not in its 
current form (but probably not at all). I suggest looking into something 
like this if you'd like to provide user-customizable styles.


Greetings,
Sven


OpenPGP_0xA4AAD0019BE03F15.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

Re: Genderidentity

[Sven Brauch]

Hi,

On 1/6/22 18:56, Nicolás Alvarez wrote:

There is a time and place to teach kids about the complexity of gender
and I don't think an exercise about arithmetic/counting is the right
place.


Additionally, there is also a time and place to discuss gender identity 
topics, but the kde-devel mailing list thread about GCompris' usage of 
the KDE i18n API is not that time and place.


Best,
Sven


OpenPGP_0xA4AAD0019BE03F15.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

Re: KDE Incubator Project Application

[Sven Brauch]

Hi Frank,

On 11/29/21 10:58, Frank Pereny wrote:
> I am interested in bringing my project to the KDE family.  I am
> requesting a sponsor, please allow me to introduce the project below.

thanks for your submission!

From a quick glance, here are a few things I spotted:

 - The application doesn't build on my system, see attachment.

 - The standard layout is to have source code in a src/ subfolder.

 - License headers are missing.

 - A lot of things are pointers for seemingly no reason. If you 
heap-allocate a std::vector, a QString or QStringList with "new", you 
should stop and ask yourself "Wait. Is this really the right thing to 
do?". (The reason is that these things are already wrapper classes 
around heap memory. Heap-allocating the wrapper data structure is rarely 
helpful)


All the best,
Sven
» make -j
[ 10%] Automatic MOC and UIC for target konverter
[ 10%] Built target konverter_autogen
[ 30%] Automatic RCC for icons.qrc
[ 30%] Generating ../konverter_en_US.ts
Scanning directory '/home/sven/Downloads/konverter'...
Updating '../konverter_en_US.ts'...
Found 27 source text(s) (0 new and 27 already existing)
[ 50%] Building CXX object CMakeFiles/konverter.dir/datatablewindow.cpp.o
[ 50%] Building CXX object CMakeFiles/konverter.dir/aboutdialog.cpp.o
[ 70%] Building CXX object 
CMakeFiles/konverter.dir/konverter_autogen/mocs_compilation.cpp.o
[ 70%] Building CXX object CMakeFiles/konverter.dir/main.cpp.o
[ 80%] Building CXX object CMakeFiles/konverter.dir/newunitdialog.cpp.o
[ 90%] Building CXX object 
CMakeFiles/konverter.dir/konverter_autogen/EWIEGA46WW/qrc_icons.cpp.o
/home/sven/Downloads/konverter/newunitdialog.cpp: In member function ‘void 
NewUnitDialog::on_buttonBox_accepted()’:
/home/sven/Downloads/konverter/newunitdialog.cpp:125:35: error: variable 
‘QTextStream stream’ has initializer but incomplete type
  125 | QTextStream stream();
  |   ^
make[2]: *** [CMakeFiles/konverter.dir/build.make:158: 
CMakeFiles/konverter.dir/newunitdialog.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs
/home/sven/Downloads/konverter/datatablewindow.cpp: In member function ‘void 
DataTableWindow::refresh_data()’:
/home/sven/Downloads/konverter/datatablewindow.cpp:135:46: error: ambiguous 
overload for ‘operator=’ (operand types are ‘QStringList’ and ‘’)
  135 | *unit_names = {"°C", "°F", "°K", "°R"};
  |  ^
In file included from /usr/include/qt/QtCore/qlist.h:1196,
 from /usr/include/qt/QtCore/qobject.h:49,
 from /usr/include/qt/QtWidgets/qwidget.h:45,
 from /usr/include/qt/QtWidgets/qmainwindow.h:44,
 from /usr/include/qt/QtWidgets/QMainWindow:1,
 from /home/sven/Downloads/konverter/datatablewindow.h:4,
 from /home/sven/Downloads/konverter/datatablewindow.cpp:1:
/usr/include/qt/QtCore/qstringlist.h:124:18: note: candidate: ‘QStringList& 
QStringList::operator=(const QList&)’
  124 | QStringList =(const QList )
  |  ^~~~
/usr/include/qt/QtCore/qstringlist.h:126:18: note: candidate: ‘QStringList& 
QStringList::operator=(QList&&)’
  126 | QStringList =(QList &) noexcept
  |  ^~~~
/usr/include/qt/QtCore/qstringlist.h:111:7: note: candidate: ‘QStringList& 
QStringList::operator=(const QStringList&)’
  111 | class QStringList : public QList
  |   ^~~
/usr/include/qt/QtCore/qstringlist.h:111:7: note: candidate: ‘QStringList& 
QStringList::operator=(QStringList&&)’
/home/sven/Downloads/konverter/datatablewindow.cpp:137:49: error: ambiguous 
overload for ‘operator=’ (operand types are ‘QStringList’ and ‘’)
  137 | *unit_notes = {"Master Unit", "", "", ""};
  | ^
In file included from /usr/include/qt/QtCore/qlist.h:1196,
 from /usr/include/qt/QtCore/qobject.h:49,
 from /usr/include/qt/QtWidgets/qwidget.h:45,
 from /usr/include/qt/QtWidgets/qmainwindow.h:44,
 from /usr/include/qt/QtWidgets/QMainWindow:1,
 from /home/sven/Downloads/konverter/datatablewindow.h:4,
 from /home/sven/Downloads/konverter/datatablewindow.cpp:1:
/usr/include/qt/QtCore/qstringlist.h:124:18: note: candidate: ‘QStringList& 
QStringList::operator=(const QList&)’
  124 | QStringList =(const QList )
  |  ^~~~
/usr/include/qt/QtCore/qstringlist.h:126:18: note: candidate: ‘QStringList& 
QStringList::operator=(QList&&)’
  126 | QStringList =(QList &) noexcept
  |  ^~~~
/usr/include/qt/QtCore/qstringlist.h:111:7: note: candidate: ‘QStringList& 
QStringList::operator=(const QStringList&)’
  111 | class QStringList : public QList
  |   ^~~
/usr/include/qt/QtCore/qstringlist.h:111:7: note: candidate: ‘QStringList& 

D29789: Make text always align with font base line

[Sven Brauch]

brauch added a comment.


  Hmm, consider though that a configuration option should be something that 
gives a choice to the user. It shouldn't be necessary to set a config option in 
order to make the program behave correctly.

REPOSITORY
  R39 KTextEditor

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

To: xuetianweng, rjvbb, dhaumann, cullmann
Cc: brauch, kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, 
domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Sven Brauch]

brauch added a comment.


  In D25339#663915 , @fakefred wrote:
  
  > I second the as-an-option proposal. Hey, why not automatically increase the 
line height when CJK characters are detected?
  
  
  This issue has been around for years and actually yeah, I think that is the 
only viable solution unless somebody dives in and reworks the renderer to 
support variable line heights.

REPOSITORY
  R39 KTextEditor

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

To: xuetianweng, #ktexteditor, cullmann, dhaumann, #frameworks, rjvbb
Cc: brauch, sars, pshinjo, rjvbb, fakefred, anthonyfieroni, 
kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, cblack, domson, 
michaelh, ngraham, bruns, demsking, cullmann, dhaumann

D27912: Draw inlineNotes after drawing word wrap marker

[Sven Brauch]

brauch added a comment.


  I also don't understand this. Even if the painting somehow changes, e.g. 
because some painter state is set which wasnt set before (which I do not see to 
be the case here), that should not affect the line layout, as that is computed 
separately.
  
  Very strange.

REPOSITORY
  R39 KTextEditor

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

To: davidre, #ktexteditor, brauch
Cc: cullmann, brauch, kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, 
cblack, GB_2, domson, michaelh, ngraham, bruns, demsking, sars, dhaumann

D27912: Draw inlineNotes after drawing word wrap marker

[Sven Brauch]

brauch accepted this revision.
brauch added a comment.
This revision is now accepted and ready to land.


  Looks good, thanks! Yes, it's an improvement.

REPOSITORY
  R39 KTextEditor

BRANCH
  caret

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

To: davidre, #ktexteditor, brauch
Cc: brauch, kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, 
GB_2, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D26840: Correctly set underMouse() for inline notes

[Sven Brauch]

This revision was automatically updated to reflect the committed changes.
Closed by commit R39:7f043fbb26d4: inline notes: correctly set underMouse() for 
inline notes (authored by brauch).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D26840?vs=74110=74351#toc

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26840?vs=74110=74351

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

AFFECTED FILES
  autotests/src/inlinenote_test.cpp
  src/view/kateview.cpp
  src/view/kateviewinternal.cpp

To: davidre, #ktexteditor, cullmann, brauch
Cc: brauch, cullmann, kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, 
GB_2, domson, michaelh, ngraham, bruns, demsking, sars, dhaumann

D26840: Correctly set underMouse() for inline notes

[Sven Brauch]

brauch added a comment.


  Ok, done ;)

REPOSITORY
  R39 KTextEditor

BRANCH
  underMouse (branched from master)

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

To: davidre, #ktexteditor, cullmann, brauch
Cc: brauch, cullmann, kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, 
GB_2, domson, michaelh, ngraham, bruns, demsking, sars, dhaumann

D26840: Correctly set underMouse() for inline notes

[Sven Brauch]

brauch accepted this revision.
brauch added a comment.


  If you want I can integrate these changes and submit your patch, should I? 
Thanks a lot for your contribution.

REPOSITORY
  R39 KTextEditor

BRANCH
  underMouse (branched from master)

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

To: davidre, #ktexteditor, cullmann, brauch
Cc: brauch, cullmann, kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, 
GB_2, domson, michaelh, ngraham, bruns, demsking, sars, dhaumann

D26840: Correctly set underMouse() for inline notes

[Sven Brauch]

brauch added a comment.


  I'm sorry, updateView is the wrong function to call, you need updateDirty. I 
tried it out, like this it works:
  
if (e->buttons() == Qt::NoButton) {
auto noteData = inlineNoteAt(e->globalPos());
if (noteData.m_position.isValid()) {
if (!m_activeInlineNote.m_position.isValid()) {
// no active note -- focus in
tagLine(noteData.m_position);
updateDirty();
noteData.m_underMouse = true;

noteData.m_provider->inlineNoteFocusInEvent(KTextEditor::InlineNote(noteData), 
e->globalPos());
m_activeInlineNote = noteData;
} else {

noteData.m_provider->inlineNoteMouseMoveEvent(KTextEditor::InlineNote(noteData),
 e->globalPos());
}
} else if (m_activeInlineNote.m_position.isValid()) {
tagLine(m_activeInlineNote.m_position);
updateDirty();
m_activeInlineNote.m_underMouse = false;

m_activeInlineNote.m_provider->inlineNoteFocusOutEvent(KTextEditor::InlineNote(m_activeInlineNote));
m_activeInlineNote = {};
}
}

REPOSITORY
  R39 KTextEditor

BRANCH
  underMouse (branched from master)

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

To: davidre, #ktexteditor, cullmann
Cc: brauch, cullmann, kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, 
GB_2, domson, michaelh, ngraham, bruns, demsking, sars, dhaumann

D26840: Correctly set underMouse() for inline notes

[Sven Brauch]

brauch added a comment.


  Then it seems like the line is not tagged correctly. Maybe try 
`tagLines(note.position.line(), note.position.line())`? I think the column 
being the same is not what the function being called expects.

REPOSITORY
  R39 KTextEditor

BRANCH
  underMouse (branched from master)

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

To: davidre, #ktexteditor, cullmann
Cc: brauch, cullmann, kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, 
GB_2, domson, michaelh, ngraham, bruns, demsking, sars, dhaumann

D26840: Correctly set underMouse() for inline notes

[Sven Brauch]

brauch added a comment.


  My guess is right now the view updates when the cursor blinks, so that's why 
it updates after a short moment (of varying length, though, if you look at the 
video). Since the line is tagged dirty, it gets repainted correctly, but too 
late.

REPOSITORY
  R39 KTextEditor

BRANCH
  underMouse (branched from master)

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

To: davidre, #ktexteditor, cullmann
Cc: brauch, cullmann, kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, 
GB_2, domson, michaelh, ngraham, bruns, demsking, sars, dhaumann

D26840: Correctly set underMouse() for inline notes

[Sven Brauch]

brauch added a comment.


  Hm, yeah, looking at the code I think you might need to call `updateView()` 
in case a focus in or out happened. Can you try if that makes a difference?

REPOSITORY
  R39 KTextEditor

BRANCH
  underMouse (branched from master)

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

To: davidre, #ktexteditor, cullmann
Cc: brauch, cullmann, kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, 
GB_2, domson, michaelh, ngraham, bruns, demsking, sars, dhaumann

D26840: Correctly set underMouse() for inline notes

[Sven Brauch]

brauch added a comment.


  Is the video the new behaviour? It still looks a bit weird to me, there is a 
slight delay between the mouse entering the area and the highlight changing.
  
  Is it possible that the line (and thus the note) is not properly marked for 
repaint when the underMouse property changes?

REPOSITORY
  R39 KTextEditor

BRANCH
  underMouse (branched from master)

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

To: davidre, #ktexteditor, cullmann
Cc: brauch, cullmann, kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, 
GB_2, domson, michaelh, ngraham, bruns, demsking, sars, dhaumann

D22511: Minimap: Do not grab left mouse click over up/down arrows

[Sven Brauch]

brauch accepted this revision.
brauch added a comment.
This revision is now accepted and ready to land.


  LGTM, thank you!

REPOSITORY
  R39 KTextEditor

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

To: sars, brauch
Cc: kwrite-devel, kde-frameworks-devel, LeGast00n, sbergeron, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D22477: With auto completion don't show completions that don't match from beginning of typed word

[Sven Brauch]

brauch added a comment.


  Also here, looks good to me, but I would wait for feedback from somebody else 
in addition. Thank you for working on this!

REPOSITORY
  R39 KTextEditor

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

To: ahmadsamir, #ktexteditor, cullmann, dhaumann, #kdevelop, kossebau, mwolff, 
kfunk
Cc: brauch, kwrite-devel, kde-frameworks-devel, LeGast00n, sbergeron, domson, 
michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D22477: With auto completion don't show completions that don't match from beginning of typed word

[Sven Brauch]

brauch added inline comments.

INLINE COMMENTS

> brauch wrote in katecompletionmodel.cpp:2029
> Maybe you want to set the flag here too?

Actually no, probably not. ;)

REPOSITORY
  R39 KTextEditor

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

To: ahmadsamir, #ktexteditor, cullmann, dhaumann, #kdevelop, kossebau, mwolff, 
kfunk
Cc: brauch, kwrite-devel, kde-frameworks-devel, LeGast00n, sbergeron, domson, 
michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D22477: With auto completion don't show completions that don't match from beginning of typed word

[Sven Brauch]

brauch added inline comments.

INLINE COMMENTS

> katecompletionmodel.cpp:2029
>  if (matchesAbbreviation(m_nameColumn, match, 
> model->matchCaseSensitivity())) {
>  matchCompletion = AbbreviationMatch;
>  }

Maybe you want to set the flag here too?

REPOSITORY
  R39 KTextEditor

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

To: ahmadsamir, #ktexteditor, cullmann, dhaumann, #kdevelop, kossebau, mwolff, 
kfunk
Cc: brauch, kwrite-devel, kde-frameworks-devel, LeGast00n, sbergeron, domson, 
michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D22500: Make keyword completion model return HideListIfAutomaticInvocation by default

[Sven Brauch]

brauch added a comment.


  Independent of anything else I think this is a very sensible change, and 
seems like an oversight / bug.
  
  One of the more core kate guys should approve, but +1 from me.

REPOSITORY
  R39 KTextEditor

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

To: ahmadsamir, #ktexteditor, #kdevelop, cullmann, dhaumann, brauch
Cc: kde-frameworks-devel, kwrite-devel, LeGast00n, sbergeron, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D22477: Add a config to only show completions matching word beginning

[Sven Brauch]

brauch added a comment.


  I'm not sure if this solves the right problem.
  
  Where I notice this issue a lot is when typing "return". The keyword 
completion suggests "return", and when I want a newline, I complete the 
"return" first. This doesn't affect C++ (because of the trailing ";"), but it 
does affect other languages.
  
  I think this is pretty much the same issue. Maybe it should be solved by 
hiding the completion window if it was not explicitly invoked if there is any 
entry matching what you currently typed?
  
  Then typing Foo would not have the completion window open at all in the 
described case (thus solving your problem, as well as mine), and if you still 
wanted to complete BarFoo, you would press Ctrl+Space.

REPOSITORY
  R39 KTextEditor

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

To: ahmadsamir, #ktexteditor, cullmann, dhaumann, #kdevelop, kossebau, mwolff, 
kfunk
Cc: brauch, kwrite-devel, kde-frameworks-devel, LeGast00n, sbergeron, domson, 
michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D20852: Fix to show folding preview when move the mouse from bottom to top

[Sven Brauch]

brauch added a comment.


  I guess the intention of the highlight delay is that when you move your mouse 
across the border, the view doesn't flicker. The 150ms does this well enough 
for me, I never see a flicker at least ;)

REPOSITORY
  R39 KTextEditor

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

To: loh.tar, #ktexteditor
Cc: brauch, kwrite-devel, kde-frameworks-devel, #ktexteditor, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D19764: Fix Minimap with QtCurve style

[Sven Brauch]

brauch accepted this revision.
brauch added a comment.
This revision is now accepted and ready to land.


  Looks sensible, and I think I've even seen this bug already somewhere.

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

D19621: ViewPrivate: Make deselection by arrow keys more handy

[Sven Brauch]

brauch added a comment.


  Sorry, yeah, I misunderstood!

REPOSITORY
  R39 KTextEditor

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

To: loh.tar, #ktexteditor
Cc: cullmann, brauch, dhaumann, ngraham, kwrite-devel, kde-frameworks-devel, 
#ktexteditor, gennad, domson, michaelh, bruns, demsking, sars

D19621: ViewPrivate: Make deselection by arrow keys more handy

[Sven Brauch]

brauch added a comment.


  Hm, so shift-selecting with Shift+Left 5 times, then pressing Shift+Right 
once clears the selection? Just for the statistics, deselecting one character 
is a feature I use all the time.
  
  Why is this behaviour helpful? If you want to clear the selection, press Esc.

REPOSITORY
  R39 KTextEditor

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

To: loh.tar, #ktexteditor
Cc: brauch, dhaumann, ngraham, kwrite-devel, kde-frameworks-devel, 
#ktexteditor, gennad, domson, michaelh, bruns, demsking, cullmann, sars

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

[Sven Brauch]

brauch added a comment.


  I would also advise against calling processEvents() to keep UIs responsive in 
all cases. It's tempting, but it is near impossible to get it right. What about 
conflicting actions, close/resize events, dbus calls, etc etc that are handled 
here?
  
  I think the right way to implement this would be something like, copy the 
required data, run the expensive S with QtConcurrent, and replace it back 
into the UI when done.

REPOSITORY
  R39 KTextEditor

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

To: loh.tar, #ktexteditor, #vdg, cullmann
Cc: brauch, cullmann, abetts, kwrite-devel, kde-frameworks-devel, #ktexteditor, 
hase, michaelh, ngraham, bruns, demsking, sars, dhaumann

D17241: Disable highlighting for lines longer than 1024 characters.

[Sven Brauch]

brauch added a comment.


  Shouldn't this change simultaneously remove the line length limit ...?

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

D7010: KSqueezedTextLabel: call updateGeometry() when text changes

[Sven Brauch]

brauch added a comment.


  For the record, I tried writing a test for this but didn't succeed and 
eventually put it aside, although the difference is easily visible in a test 
application. There must be a reason why the naive test case behaves differently 
from an interactive application ... I could take another look, I guess.

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

To: brauch, cfeck, dfaure
Cc: dfaure, dhaumann, aacid, #frameworks, michaelh, ngraham, bruns

D14897: InlineNote: Pimpl inline note data without allocs

[Sven Brauch]

brauch added a comment.


  Sorry, never mind -- that code I removed yesterday.  All should be fine.

REPOSITORY
  R39 KTextEditor

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

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

D14897: InlineNote: Pimpl inline note data without allocs

[Sven Brauch]

brauch added a comment.


  Looks ok to me, except one thing: the operator== is used to compare a note 
from the list to the "currently active" note in the view. If this compares also 
the "under mouse" state, this code might be broken now ...?

REPOSITORY
  R39 KTextEditor

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

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

D14894: [RFC] Fix block mode for multi-cursor branch

[Sven Brauch]

brauch added a comment.


  Thanks for the patch, the approach looks reasonable at a first glance. You 
might want to unite the up/down functions ...
  It is too much of a WiP to merge it like this, though.

REPOSITORY
  R39 KTextEditor

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

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

D14826: inline note interface wip #2

[Sven Brauch]

brauch closed this revision.
brauch added a comment.


  commit 4ea5fee0afe5c76bbee07563c23ede808aa059de
  Author: Sven Brauch 
  Date:   Tue Aug 14 12:31:31 2018 +0200
  
Add inline note interface

Original patch by Michal Srb.

The inline note interface provides a way to render arbitrary things in
the text. The layout of the line is adapted to create space for the note.

Possible applications include showing a name of a function parameter on call
side or rendering square with color preview next to CSS color property.

Subscribers: kwrite-devel, kde-frameworks-devel

Tags: #kate, #frameworks

Differential Revision: https://phabricator.kde.org/D14826

REPOSITORY
  R39 KTextEditor

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

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

D14826: inline note interface wip #2

[Sven Brauch]

brauch updated this revision to Diff 39889.
brauch added a comment.


  update license text

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14826?vs=39888=39889

BRANCH
  inlinenotes

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

AFFECTED FILES
  src/document/katedocument.h
  src/include/CMakeLists.txt
  src/include/ktexteditor/inlinenote.h
  src/include/ktexteditor/inlinenoteinterface.h
  src/include/ktexteditor/inlinenoteprovider.h
  src/render/katerenderer.cpp
  src/render/katerenderer.h
  src/utils/ktexteditor.cpp
  src/view/kateview.cpp
  src/view/kateview.h
  src/view/kateviewinternal.cpp
  src/view/kateviewinternal.h

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

D14826: inline note interface wip #2

[Sven Brauch]

brauch updated this revision to Diff 39888.
brauch marked 21 inline comments as done.
brauch added a comment.


  Implement Dominik's suggestions

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14826?vs=39815=39888

BRANCH
  inlinenotes

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

AFFECTED FILES
  src/document/katedocument.h
  src/include/CMakeLists.txt
  src/include/ktexteditor/inlinenote.h
  src/include/ktexteditor/inlinenoteinterface.h
  src/include/ktexteditor/inlinenoteprovider.h
  src/render/katerenderer.cpp
  src/render/katerenderer.h
  src/utils/ktexteditor.cpp
  src/view/kateview.cpp
  src/view/kateview.h
  src/view/kateviewinternal.cpp
  src/view/kateviewinternal.h

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

D14826: inline note interface wip #2

[Sven Brauch]

brauch updated this revision to Diff 39815.
brauch added a comment.


  address Dominik's suggestion and split focus handling and click handling

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14826?vs=39802=39815

BRANCH
  inlinenotes

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

AFFECTED FILES
  CMakeLists.txt
  src/document/katedocument.h
  src/include/CMakeLists.txt
  src/include/ktexteditor/inlinenoteinterface.h
  src/render/katerenderer.cpp
  src/render/katerenderer.h
  src/utils/ktexteditor.cpp
  src/view/kateview.cpp
  src/view/kateview.h
  src/view/kateviewinternal.cpp
  src/view/kateviewinternal.h

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

D14826: inline note interface wip #2

[Sven Brauch]

brauch updated this revision to Diff 39802.
brauch added a comment.


  I added the rest of the interaction interface (click, mouseover)
  and reduced the API a bit by moving a few hints into the InlineNote
  object.
  
  Only thing I still intend to change about the API would be that
  we use the Qt mouse button enum to provide details about mouse events,
  otherwise nothing comes to mind.

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14826?vs=39721=39802

BRANCH
  inlinenotes

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

AFFECTED FILES
  CMakeLists.txt
  src/document/katedocument.h
  src/include/CMakeLists.txt
  src/include/ktexteditor/inlinenoteinterface.h
  src/render/katerenderer.cpp
  src/render/katerenderer.h
  src/utils/ktexteditor.cpp
  src/view/kateview.cpp
  src/view/kateview.h
  src/view/kateviewinternal.cpp
  src/view/kateviewinternal.h

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

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

[Sven Brauch]

brauch added a comment.
Herald added a project: Kate.
Herald edited subscribers, added: kde-frameworks-devel; removed: Frameworks.


  Can't we simply update our shipped schemas, and expect users with custom 
schemas to fix them?

REPOSITORY
  R39 KTextEditor

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

To: intelfx, #kdevelop, #ktexteditor, #kate, mwolff
Cc: kde-frameworks-devel, brauch, dhaumann, mwolff, kwrite-devel, michaelh, 
kevinapavew, ngraham, bruns, demsking, cullmann, sars, #frameworks

D14826: inline note interface wip #2

[Sven Brauch]

brauch updated this revision to Diff 39721.
brauch added a comment.


  add noteActivated notifier function
  
  When a note is mouse-overed or clicked, a function on the note
  provider is called, giving the point it was hovered or clicked
  in note coordinates, the type of event, and the note the event
  occured on.
  
  This can be used to e.g. expand notes on mouse-over, show a tooltip,
  or execute actions when clicking. You should even be able to e.g.
  highlight pseudo-buttons mouseover.

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14826?vs=39692=39721

BRANCH
  inlinenotes

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

AFFECTED FILES
  CMakeLists.txt
  src/document/katedocument.h
  src/include/ktexteditor/inlinenoteinterface.h
  src/render/katerenderer.cpp
  src/render/katerenderer.h
  src/utils/ktexteditor.cpp
  src/view/kateview.cpp
  src/view/kateview.h
  src/view/kateviewinternal.cpp
  src/view/kateviewinternal.h

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

D14826: inline note interface wip #2

[Sven Brauch]

brauch added a comment.


  Thanks for the feedback! I will try doing a few more things with this 
interace and then maybe discuss again with the other kate people here at 
Akademy about which one they like better.
  
  About the tracking, I don't think anything is needed on the side of the 
interface. You can see how you can potentially do it in the KDevelop patch I 
attached: create a moving cursor for the location you want the note to track, 
and then compute the note's position from the moving cursor's position as 
needed in the getter each time.
  I think this is even better than doing it in the interface itself, since it 
is more flexible with regards to how exactly the moving cursor behaves.
  
  Regarding the QVarLengthArray, performance-wise it would be better, but it 
makes the public API ugly (QVarLengthArray is a relatively internal, hacky 
class which is supposed to be only used in special cases), so I think we should 
first profile whether this is a bottleneck in any possible use case (it 
probably isn't).
  
  Currently, I'm trying out whether we can add some simple interaction ("note 
clicked") as well already, since I think that would be nice long-term. If the 
API is nice, we can fix small uglyness like the cursor navigation around notes 
at any later time IMHO.
  
  Best,
  Sven

REPOSITORY
  R39 KTextEditor

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

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

D14826: inline note interface wip #2

[Sven Brauch]

brauch updated this revision to Diff 39692.
brauch added a comment.


  add missing files

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14826?vs=39682=39692

BRANCH
  inlinenotes

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

AFFECTED FILES
  src/document/katedocument.h
  src/include/CMakeLists.txt
  src/include/ktexteditor/inlinenoteinterface.h
  src/render/katerenderer.cpp
  src/utils/ktexteditor.cpp
  src/view/kateview.cpp
  src/view/kateview.h

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

D14826: inline note interface wip #2

[Sven Brauch]

brauch added a comment.


  Sample patch for KDevelop's problem highlighter plus screenshot:
  F6192637: hl.png 
  F6192639: inline-problems.diff 

REPOSITORY
  R39 KTextEditor

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

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

D14826: inline note interface wip #2

[Sven Brauch]

brauch edited the summary of this revision.

REPOSITORY
  R39 KTextEditor

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

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

D14826: inline note interface wip #2

[Sven Brauch]

brauch added reviewers: michalsrb, dhaumann, cullmann.

REPOSITORY
  R39 KTextEditor

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

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

D14826: inline note interface wip #2

[Sven Brauch]

brauch created this revision.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
brauch requested review of this revision.

REPOSITORY
  R39 KTextEditor

BRANCH
  master

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

AFFECTED FILES
  src/document/katedocument.h
  src/include/CMakeLists.txt
  src/render/katerenderer.cpp
  src/utils/ktexteditor.cpp
  src/view/kateview.cpp
  src/view/kateview.h

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

D12662: Add InlineNoteInterface

[Sven Brauch]

brauch added a comment.


  I'd like to play with this a bit wrt what can be done in KDevelop with it (I 
want the problem popups gone). Would you mind if I do some changes along the 
way? I would post an updated patch here, in case I actually come up with useful 
changes ...

REPOSITORY
  R39 KTextEditor

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

To: michalsrb, #ktexteditor
Cc: kwrite-devel, kde-frameworks-devel, dhaumann, cullmann, ngraham, brauch, 
michaelh, kevinapavew, bruns, demsking, sars

D14758: use view lines for wheel scrolling, not real lines

[Sven Brauch]

This revision was automatically updated to reflect the committed changes.
Closed by commit R39:2815fea7854f: Fix: Scroll view lines instead of real lines 
for wheel and touchpad scrolling (authored by brauch).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D14758?vs=39499=39561#toc

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14758?vs=39499=39561

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

AFFECTED FILES
  src/view/kateviewinternal.cpp
  src/view/kateviewinternal.h

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

D14773: completion widget: fix minimum header section size

[Sven Brauch]

This revision was automatically updated to reflect the committed changes.
Closed by commit R39:5499a0df825c: completion widget: fix minimum header 
section size (authored by brauch).

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14773?vs=39533=39562

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

AFFECTED FILES
  src/completion/expandingtree/expandingtree.cpp

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

D12662: Add InlineNoteInterface

[Sven Brauch]

brauch added a comment.


  By the way, other people around here are very impressed by this patch as 
well, and we'd really like to get this merged :)
  Moving e.g. KDevelop's warning markers into an inline note instead of the 
annoying popup would make a real difference for usability ...

REPOSITORY
  R39 KTextEditor

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

To: michalsrb, #ktexteditor
Cc: kwrite-devel, kde-frameworks-devel, dhaumann, cullmann, ngraham, brauch, 
michaelh, kevinapavew, bruns, demsking, sars

D12662: Add InlineNoteInterface

[Sven Brauch]

brauch added a comment.


  Wow, that looks amazing! Really impressive.
  
  Using moving cursors for this sounds good to me. Other things which cannot be 
updated in real-time do that as well (highlighting, error underlines, ...)
  
  Regarding the cursor issue: I think you would need a state variable whether 
the cursor is left or right of the note. You could then patch 
KateViewInternal::placeCursor to set this correctly and also the cursor move 
functions to take this into account. That's not ideal since it requires to 
modify a few places, but probably actually adding a character would require 
more modifications...

REPOSITORY
  R39 KTextEditor

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

To: michalsrb, #ktexteditor
Cc: kwrite-devel, kde-frameworks-devel, dhaumann, cullmann, ngraham, brauch, 
michaelh, kevinapavew, bruns, demsking, sars

D14758: use view lines for wheel scrolling, not real lines

[Sven Brauch]

brauch added a comment.


  In D14758#307401 , @ngraham wrote:
  
  > In D14758#307333 , @brauch wrote:
  >
  > > The "overly sensitive touchpad" issue seems to be missing accumulation of 
scroll events, so this patch to my understanding should not have it.
  >
  >
  > Yeah, I don't think that's our bug, and we shouldn't work around it here. 
The reporter should file a bug against libinput.
  
  
  Hmm, I understood this differently. I thought this happens when you react to 
each input event sent by the kernel, regardless of its delta, like e.g. the old 
gwenview code did which you linked: it just zooms in once per event. Thus 
devices which generate more events with smaller deltas zoom in faster, which is 
not good. And this bug is definitely at the application level, not in libinput.

REPOSITORY
  R39 KTextEditor

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

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

D12662: Add InlineNoteInterface

[Sven Brauch]

brauch added a comment.
Restricted Application edited subscribers, added: kde-frameworks-devel, 
kwrite-devel; removed: Frameworks.


  Hey there, what's happening to this? I think this is a really nice feature 
and it would be very sad if it would bitrot :(
  Anything anyone can assist with? We are currently at Akademy, KDE's annual 
conference, so we would have time to discuss any issues right now.

REPOSITORY
  R39 KTextEditor

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

To: michalsrb, #ktexteditor
Cc: kwrite-devel, kde-frameworks-devel, dhaumann, cullmann, ngraham, brauch, 
michaelh, kevinapavew, bruns, demsking, sars, #frameworks

D14758: use view lines for wheel scrolling, not real lines

[Sven Brauch]

brauch added a comment.


  The "overly sensitive touchpad" issue seems to be missing accumulation of 
scroll events, so this patch to my understanding should not have it.

REPOSITORY
  R39 KTextEditor

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

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

D14758: use view lines for wheel scrolling, not real lines

[Sven Brauch]

brauch added a comment.


  I will modify the commit message as suggested.
  
  I read through the bug report, and I think this patch does fix the main issue 
discussed there. The patch suggested by one user there is even quite similar 
(if a bit less complete) than this one. The original complaint (touchpad vs. 
mouse wheel) to me personally sounds a bit strange and I'm not sure if that is 
actually the point. I'm not even sure if that is what the original reporter was 
in fact annoyed by (in contrast to "what he wrote he was annoyed by").

REPOSITORY
  R39 KTextEditor

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

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

D14773: completion widget: fix minimum header section size

[Sven Brauch]

brauch created this revision.
brauch added a reviewer: Kate.
Restricted Application added projects: Kate, Frameworks.
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel.
brauch requested review of this revision.

REVISION SUMMARY
  The header is hidden, but if the minimum size is -1 (the default), Qt
  chooses something non-zero as the minimum size. The completion widget
  then sets it to something, assumes it has that width, and messes up
  its layout when it actually ends up being larger than that.

REPOSITORY
  R39 KTextEditor

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

AFFECTED FILES
  src/completion/expandingtree/expandingtree.cpp

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

D14758: use view lines for wheel scrolling, not real lines

[Sven Brauch]

brauch created this revision.
brauch added a reviewer: dhaumann.
Restricted Application added projects: Kate, Frameworks.
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel.
brauch requested review of this revision.

REVISION SUMMARY
  I don't think there is an ideal solution for this problem, this one has the 
disadvantage that the code handling wheel scrolling is not related to the 
scroll bar any more. It still seems like the simplest one. What do you think?

REPOSITORY
  R39 KTextEditor

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

AFFECTED FILES
  src/view/kateviewinternal.cpp
  src/view/kateviewinternal.h

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

Re: Semantic analysis KDevelop

[Sven Brauch]

Hi,

Note: our mailing list is kdeve...@kde.org for user questions, or 
kdevelop-de...@kde.org for development; kde-devel@kde.org is a more general KDE 
development list and not for KDevelop specifically. Please direct 
KDevelop-specific questions to one of these lists.


On 06/14/2018 12:40 AM, Maxime. Haselbauer wrote:
Am I the only one having problem since month with the semantic analysis of 
Kdevelop


You are not the only one, but it does not affect everyone either :/
To me at least, it is not fully clear what circumstances result in this problem, 
but I think there are multiple possible reasons, all somehow related to which 
standard includes are parsed in what mode.


Best,
Sven

D13442: Implemented displaying of total lines in kate

[Sven Brauch]

brauch added a comment.


  I was also thinking about the context menu, yes. That is also where a lot of 
users would first look for it I think, even before the config dialog.

REPOSITORY
  R39 KTextEditor

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

To: shubham, #ktexteditor, cullmann, brauch
Cc: mludwig, zhigalin, ngraham, dhaumann, kwrite-devel, kde-frameworks-devel, 
michaelh, kevinapavew, bruns, demsking, head7, cullmann, kfunk, sars

D13442: Implemented displaying of total lines in kate

[Sven Brauch]

brauch added a comment.


  I don't like this being user-controllable. We spend so much effort in keeping 
our option set clearly arranged and limited to useful choices, to me this seems 
like a very bad opportunity to break that rule.

REPOSITORY
  R39 KTextEditor

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

To: shubham, #ktexteditor, cullmann, brauch
Cc: mludwig, zhigalin, ngraham, dhaumann, kwrite-devel, kde-frameworks-devel, 
michaelh, kevinapavew, bruns, demsking, head7, cullmann, kfunk, sars

D13442: Implemented displaying of total lines in kate

[Sven Brauch]

brauch added a comment.


  Since the other two number displays use QLocale().toString(), probaly this 
should too ...

REPOSITORY
  R39 KTextEditor

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

To: shubham, #ktexteditor, cullmann, brauch
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, head7, cullmann, kfunk, sars, dhaumann

D13365: Fixed the cursor(caret) width in kate

[Sven Brauch]

This revision was automatically updated to reflect the committed changes.
Closed by commit R39:e6f87dd57008: Fix caret width (authored by shubham, 
committed by brauch).

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13365?vs=35683=35735

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

AFFECTED FILES
  src/render/katerenderer.cpp

To: shubham, #ktexteditor, brauch, cullmann
Cc: cullmann, ngraham, brauch, kwrite-devel, kde-frameworks-devel, michaelh, 
kevinapavew, bruns, demsking, sars, dhaumann

D13365: Fixed the cursor(caret) width in kate

[Sven Brauch]

brauch added a comment.


  I'm sorry I am so annoying, but iirc KDE's commit hooks will not accept 
commits with only one name fragment. Something equivalent to the western 
first/last name pair is required.

REPOSITORY
  R39 KTextEditor

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

To: shubham, #ktexteditor, brauch, cullmann
Cc: cullmann, ngraham, brauch, kwrite-devel, kde-frameworks-devel, michaelh, 
kevinapavew, bruns, demsking, sars, dhaumann

D13365: Fixed the cursor(caret) width in kate

[Sven Brauch]

brauch added a comment.


  To submit the change with your name on it, I'd need a full name and email 
address, can you provide that? Thanks!

REPOSITORY
  R39 KTextEditor

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

To: shubham, #ktexteditor, brauch, cullmann
Cc: cullmann, ngraham, brauch, kwrite-devel, kde-frameworks-devel, michaelh, 
kevinapavew, bruns, demsking, sars, dhaumann

D13365: Fixed the cursor(caret) width in kate

[Sven Brauch]

brauch added a comment.


  Yes that looks better, I'll submit it later today.

REPOSITORY
  R39 KTextEditor

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

To: shubham, #ktexteditor, brauch, cullmann
Cc: cullmann, ngraham, brauch, kwrite-devel, kde-frameworks-devel, michaelh, 
kevinapavew, bruns, demsking, sars, dhaumann

D13365: Fixed the cursor(caret) width in kate

[Sven Brauch]

brauch added a comment.


  Click "download raw diff" at the top-right of the page. This is the diff you 
uploaded. This is not the change we want to apply when merging this review 
request; it only contains the changes you did from your earlier version to the 
latest one, not the set of changes which needs to be applied to the repository. 
If you try to apply this patch to ktexteditor master, you will see that it 
fails.

REPOSITORY
  R39 KTextEditor

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

To: shubham, #ktexteditor, brauch, cullmann
Cc: cullmann, ngraham, brauch, kwrite-devel, kde-frameworks-devel, michaelh, 
kevinapavew, bruns, demsking, sars, dhaumann

D13365: Fixed the cursor(caret) width in kate

[Sven Brauch]

brauch added a comment.


  The change is good now but you screwed up the patch: you need to submit the 
full set of changes you want to do as a patch, not only the last iteration. Can 
you fix that? Thank you!

REPOSITORY
  R39 KTextEditor

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

To: shubham, #ktexteditor, brauch, cullmann
Cc: cullmann, ngraham, brauch, kwrite-devel, kde-frameworks-devel, michaelh, 
kevinapavew, bruns, demsking, sars, dhaumann

D13365: Fixed the cursor(caret) width in kate

[Sven Brauch]

brauch accepted this revision.
brauch added a comment.
This revision is now accepted and ready to land.


  Yep, with that, it looks good to me. Do you have commit access?

REPOSITORY
  R39 KTextEditor

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

To: shubham, #ktexteditor, brauch
Cc: ngraham, brauch, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, 
bruns, demsking, cullmann, sars, dhaumann

D13365: BUG:391518 Fixed the cursor(caret) width in kate

[Sven Brauch]

brauch added a comment.


  Good find (annoyed me too) and sounds plausible to me. Thanks!
  
  Just one thing, do you call setRenderHint() inside the save() / restore() 
pair of the painter so the previous state is restored after drawing the caret?

REPOSITORY
  R39 KTextEditor

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

To: shubham, #ktexteditor
Cc: brauch, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, 
bruns, demsking, cullmann, sars, dhaumann

D13365: BUG:391518 Fixed the cursor(caret) width in kate

[Sven Brauch]

brauch added a comment.


  Good find (annoyed me too) and sounds plausible to me. Thanks!
  
  Just one thing, do you call setRenderHint() inside the save() / restore() 
pair of the painter so the previous state is restored after drawing the caret?

REPOSITORY
  R39 KTextEditor

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

To: shubham, #ktexteditor
Cc: brauch, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, 
bruns, demsking, cullmann, sars, dhaumann

D12768: Allow wrapping selection off top/bottom of autocomplete results

[Sven Brauch]

brauch added a comment.


  What happens if the overload selection window is open in addition (like in 
KDevelop)?

REPOSITORY
  R39 KTextEditor

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

To: sraizada, #ktexteditor
Cc: brauch, #ktexteditor, #frameworks, michaelh, kevinapavew, ngraham, bruns, 
demsking, head7, cullmann, kfunk, sars, dhaumann

D12662: Add InlineNoteInterface

[Sven Brauch]

brauch added inline comments.

INLINE COMMENTS

> dhaumann wrote in katedocument.cpp:5316-5320
> Btw, may I am wrong here, since this is really called often when painting 
> lines... So this may be good to have.

I also stumbled upon it but it avoids an alloc in a very deep loop, so it might 
be worth it here.

REPOSITORY
  R39 KTextEditor

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

To: michalsrb, #ktexteditor
Cc: dhaumann, cullmann, ngraham, brauch, #frameworks, michaelh, kevinapavew, 
bruns, demsking, sars

D12662: Add InlineNoteInterface

[Sven Brauch]

brauch added a comment.


  Oh, heh, yes it does. It's just your docstring which says otherwise ("is an 
interface for the View"), and that's what I looked at at that time. That should 
be changed. ;)
  
  I think the question is less what you need and more what it is supposed to do 
semantically. Do you add annotations to a *document*, or do you add them to a 
specific *view* of that document (remember that there can be more than one view 
of a document, even visible at once, e.g. splitview)? I think you can find 
arguments for both, but I feel like the document variant (as you have it) will 
be easier to handle.

REPOSITORY
  R39 KTextEditor

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

To: michalsrb, #ktexteditor
Cc: cullmann, ngraham, brauch, #frameworks, michaelh, kevinapavew, bruns, 
demsking, sars, dhaumann

D12662: Add InlineNoteInterface

[Sven Brauch]

brauch added a comment.


  I think fixing the selection rendering issue would be nice.
  
  Regarding blockmode behaviour, I think it will be a bit strange either way. 
One thing to compare is the behaviour with non-fixed width fonts: there block 
mode also selects constant columns (i.e. it will look ragged). This is probably 
closest to what you want when you use block mode, so I think it should stay the 
way you have it now.
  
  Regarding linewrap, I think what at least must always be the case is that 
with dynwrap on, all text is visible. That notes cannot be placed nicely is 
probably unavoidable in extreme cases. I guess the best behaviour would be to 
just wrap the notes as if they were part of the text, with the restriction that 
you cannot split them up ...
  
  I would not rely on the user of the interface placing his notes in a visible 
column. If e.g. KDevelop would place some notes, and then the user resizes his 
window, and then they need to be re-placed by the interface user? That seems 
strange. Consider also that you can have a split view with two different widths 
for the same file and stuff like that.
  
  Oh, good point by the way: Are you sure this should be an interface to a 
*view*? Maybe it should instead be attached to the document? I'm not sure, I 
just want to bring the discussion up.

REPOSITORY
  R39 KTextEditor

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

To: michalsrb, #ktexteditor
Cc: ngraham, brauch, #frameworks, michaelh, kevinapavew, bruns, demsking, 
cullmann, sars, dhaumann

D12662: Add InlineNoteInterface

[Sven Brauch]

brauch added a comment.


  Looks good from the implementation too so far. One thing I do not see is any 
changes to the cursorToX / xToCursor functions, is there really no change 
required there?
  
  Some things which come to my mind for testing would be:
  
  - is selection rendered correctly if it includes notes, at the end, 
beginning, or middle of lines, also mult-line selections?
  - what happens when clicking or dragging from or into the notes?
  - does it still work properly with dynamic word-wrap on?
  - does it work properly with code-folding? what happens if a note is at the 
border of a folding region?

INLINE COMMENTS

> katedocument.cpp:5295
> +
> +connect(provider, SIGNAL(reset()), this, SLOT(inlineNotesReset()));
> +connect(provider, SIGNAL(lineChanged(int)), this, 
> SLOT(inlineNotesLineChanged(int)));

Can you use new-style connect here, i.e. the function-pointer syntax
connect(provider, ::reset, this, 
::inlineNotesReset)?
This gives compile-time argument type checking.

> katerenderer.cpp:771
> +// Determine the position where to paint the note.
> + // We start by getting the x coordinate of cursor placed to the 
> column.
> +qreal x = 
> range->viewLine(viewLine).lineLayout().cursorToX(column) - xStart;

indent

> katerenderer.cpp:777
> +// note should be painted and the cursor gets placed at the 
> right side of it. So we have to
> + // subtract the width of the note to get to left side of the 
> hole.
> +x -= inlineNote->width(lineHeight(), currentFontMetrics());

indent

REPOSITORY
  R39 KTextEditor

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

To: michalsrb, #ktexteditor
Cc: brauch, #frameworks, michaelh, kevinapavew, ngraham, bruns, demsking, 
cullmann, sars, dhaumann

D12662: Add InlineNoteInterface

[Sven Brauch]

brauch added a comment.


  Awesome idea! Do you have a screenshot of how it looks?

REPOSITORY
  R39 KTextEditor

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

To: michalsrb, #ktexteditor
Cc: brauch, #frameworks, michaelh, kevinapavew, ngraham, bruns, demsking, 
cullmann, sars, dhaumann

D12016: [ktexteditor] much faster positionFromCursor

[Sven Brauch]

brauch added a comment.


  Oh and, you do not need to inherit QObject to use connect; you can connect to 
a lambda calling the member function AFAIK or so. Just omit the third argument 
in connect(). What you lose by doing this is the automatic disconnect of the 
connection when the receiver object is deleted, so make sure that doesn't 
happen.

REPOSITORY
  R39 KTextEditor

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

To: jtamate, #kate, cullmann, #frameworks
Cc: brauch, cullmann, #frameworks, michaelh, kevinapavew, ngraham, bruns, 
demsking, sars, dhaumann

D12016: [ktexteditor] much faster positionFromCursor

[Sven Brauch]

brauch added a comment.


  Re. binary compatibility: should be fine because this class is not exported 
(no KTEXTEDITOR_EXPORT macro).

REPOSITORY
  R39 KTextEditor

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

To: jtamate, #kate, cullmann, #frameworks
Cc: brauch, cullmann, #frameworks, michaelh, kevinapavew, ngraham, bruns, 
demsking, sars, dhaumann

D11838: Turn on line numbers by default

[Sven Brauch]

brauch added a comment.


  For KDevelop this is fine, I don't think we have any objection.

REPOSITORY
  R39 KTextEditor

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

To: ngraham, #kate, #ktexteditor, dhaumann, mludwig
Cc: brauch, mludwig, kfunk, dhaumann, #frameworks, michaelh, kevinapavew, 
ngraham, demsking, cullmann, sars

D11811: avoid Asan runtime error: shift exponent -1 is negative

[Sven Brauch]

brauch added a comment.


  Change looks good (the previous code definitely looks like nonsense), but 
what does this mean for existing settings, saved previously?

REPOSITORY
  R39 KTextEditor

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

To: jtamate, #kate, #frameworks
Cc: brauch, michaelh, kevinapavew, ngraham, demsking, cullmann, sars, dhaumann

Re: GSoC2018 - Panel Improvement Proposal Idea

[Sven Brauch]

Hi,

On 03/25/18 14:11, Furkan Tokaç wrote:
> For GSoC2018, as a long-term KDE user/lover, I want to make 2 important 
> improvements on KDE / Plasma-Workspace / Panel that will increase user 
> experience.

Both things sound like worthwhile fixes, but are not suitable as-is for
a GSoC propsal. They could be bullet points in a proposal like "polish
the panel user experience" or similar, but as you describe it, it's two
little bug fixes which might be done with two lines of code each. That
is not a suitable scope for a project spanning 3 full man-months of
work, so I would recommend embedding them into a much broader project idea.

Best of luck,
Sven

P.S. since I read this so often: it's "improve user experience", not
"increase" ;)



signature.asc
Description: OpenPGP digital signature

Re: Q_ASSERT(!FalseSecurity)

[Sven Brauch]

Hi,

On 03/10/2018 09:53 AM, Michael Heidelbach wrote:
> Am I getting something wrong? Or is
> 
>    "Q_ASSERT(m_writeTrans);
> 
>    m_writeTrans->commit();"
> 
> providing false security?

a lot of KDE code is written this way. It will end up crashing in both
debug and release, but in debug the message will be clearer. I think
this makes sense for cases where you think it is very unlikely that the
condition will not be met. I also think adding the conditional
everywhere often makes debugging harder because it prevents the crash.

Best,
Sven



signature.asc
Description: OpenPGP digital signature

D9569: Fix wildcard matching for modelines

[Sven Brauch]

brauch accepted this revision.
brauch added a comment.
This revision is now accepted and ready to land.


  Heh. Fix looks obviously correct to me (good find), and tests are always nice.

REPOSITORY
  R39 KTextEditor

BRANCH
  TestModelines (branched from master)

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

To: dhaumann, cullmann, kfunk, mwolff, nalvarez, brauch
Cc: brauch, #frameworks, kevinapavew, ngraham, demsking, cullmann, sars, 
dhaumann

D7660: Fix a regression caused by changing backspace key behavior

[Sven Brauch]

brauch reopened this revision.
brauch added inline comments.

INLINE COMMENTS

> katedocument_test.cpp:452
>  auto view = 
> static_cast(doc.createView(nullptr));
> +view.config()->setBackspaceRemoveComposed(true);
>  doc.setText(QString::fromUtf8("व्यक्तियों"));

must be view->config ... how did this patch ever compile for you??

> katedocument.cpp:3190
> +}
> +if (!config()->backspaceIndents() || pos) {
> +KTextEditor::Cursor beginCursor(line, 0);

pos is not defined at this place

REPOSITORY
  R39 KTextEditor

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

To: safaalfulaij, #ktexteditor, jgrulich, hein, dhaumann, cullmann
Cc: brauch, mwolff, ngraham, anthonyfieroni, cullmann, jgrulich, dhaumann, 
hein, kwrite-devel, #frameworks

D7660: Fix a regression caused by changing backspace key behavior

[Sven Brauch]

brauch added a comment.


  After this was submitted master doesn't compile for me, and if I fix the 
compile in the trivial way the test fails. Can you have another look?

REPOSITORY
  R39 KTextEditor

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

To: safaalfulaij, #ktexteditor, jgrulich, hein, dhaumann, cullmann
Cc: brauch, mwolff, ngraham, anthonyfieroni, cullmann, jgrulich, dhaumann, 
hein, kwrite-devel, #frameworks

Re: releaseme new requirement: non-conflicting files on case-insensitive FS/OS

[Sven Brauch]

On 21/11/17 03:53, Felix Miata wrote:
> Case sensitive filesystems are one of the most annoying things about FOSS. 

I'd count that as a win.



signature.asc
Description: OpenPGP digital signature

Re: Do not enforce copy and paste restrictions in Okular

[Sven Brauch]

On 12/11/17 11:37, dennis knorr wrote:
> This should be at least configurable with a for-user non-changeable
> configuration. It's perfectly okay for homeowners to disable
> drm-behaviour, but there might be requirements in an enterprise context
> where that behaviour is needed.

Well, there's a checkbox in the settings ...



signature.asc
Description: OpenPGP digital signature

Re: Latte : make_unique for gcc <=4.8

[Sven Brauch]

On 05/11/17 16:12, Michail Vourlakos wrote:
> Do you know any better way to handle this?

You can let cmake do the check:

https://cmake.org/cmake/help/v3.0/module/CheckSymbolExists.html

Not sure if this is the best option, though.

Greetings,
Sven



signature.asc
Description: OpenPGP digital signature

D8333: fix some indenters from randomly invoking indent

[Sven Brauch]

This revision was automatically updated to reflect the committed changes.
Closed by commit R39:aeebeadb5f59: fix some indenters from indenting on random 
characters (authored by brauch).

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8333?vs=20861=20874

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

AFFECTED FILES
  src/script/kateindentscript.cpp

To: brauch, #ktexteditor, cullmann, dhaumann
Cc: anthonyfieroni, #frameworks, kevinapavew, cullmann, sars, dhaumann

D8333: fix some indenters from randomly invoking indent

[Sven Brauch]

brauch marked 2 inline comments as done.
brauch added a comment.


  Yes, at least the ones I'm aware of. Thanks for the review, I'll push it in a 
moment.

REPOSITORY
  R39 KTextEditor

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

To: brauch, #ktexteditor, cullmann, dhaumann
Cc: anthonyfieroni, #frameworks, kevinapavew, cullmann, sars, dhaumann

D8333: fix some indenters from randomly invoking indent

[Sven Brauch]

brauch marked an inline comment as done.
brauch added inline comments.

INLINE COMMENTS

> anthonyfieroni wrote in kateindentscript.cpp:47
> But if triggerCharacters are undefined this variable should be false, no?

No, that's still fine, this variable just caches reading the value from the 
script object. If the script doesn't define it, it is read as empty, and then 
doesn't look again (because that won't change in the future).

REPOSITORY
  R39 KTextEditor

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

To: brauch, #ktexteditor, cullmann, dhaumann
Cc: anthonyfieroni, #frameworks, kevinapavew, cullmann, sars, dhaumann

D8333: fix some indenters from randomly invoking indent

[Sven Brauch]

brauch created this revision.
brauch added reviewers: KTextEditor, cullmann, dhaumann.
Restricted Application added projects: Kate, Frameworks.
Restricted Application added a subscriber: Frameworks.

REVISION SUMMARY
  If triggerCharacters was not set, toString() would return "undefined",
  making indenters trigger on u, n, d, e, f, i and n.

TEST PLAN
  Trigger chars are still set correctly for e.g. cmake.

REPOSITORY
  R39 KTextEditor

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

AFFECTED FILES
  src/script/kateindentscript.cpp

To: brauch, #ktexteditor, cullmann, dhaumann
Cc: #frameworks, kevinapavew, cullmann, sars, dhaumann

Re: How to start contributing?

[Sven Brauch]

On 29/09/17 20:38, rhkra...@gmail.com wrote:
>> And since when is Scintilla a KDE project?
> 
> Never, as far as I know, but it is open source.

It's a bit weird to recommend contributing to this on kde-devel@kde.org,
given that KDE has its own text editor component ...



signature.asc
Description: OpenPGP digital signature

D7010: KSqueezedTextLabel: call updateGeometry() when text changes

[Sven Brauch]

brauch added a comment.


  I can write a test if you think this helps. I think reading the docs it is 
quite clear we must call updateGeometry() here: our sizeHint() changes when 
changing the text.

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

To: brauch, cfeck, rkflx
Cc: dhaumann, aacid, #frameworks

D7010: KSqueezedTextLabel: call updateGeometry() when text changes

[Sven Brauch]

brauch updated this revision to Diff 17638.
brauch added a comment.


  Right. Better call it here.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7010?vs=17411=17638

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

AFFECTED FILES
  src/ksqueezedtextlabel.cpp

To: brauch, cfeck
Cc: aacid, #frameworks

D7010: KSqueezedTextLabel: call updateGeometry() when text changes

[Sven Brauch]

brauch added a comment.


  With enough dedication, probably yes ...

REPOSITORY
  R236 KWidgetsAddons

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

To: brauch, cfeck
Cc: aacid, #frameworks