rjvbb marked 4 inline comments as done.
rjvbb added inline comments.

INLINE COMMENTS

> kfunk wrote in backtracegenerator.cpp:62
> Please do this only if `lldb` is used then. No need  to change code which 
> apparently worked fine for years. Also, did you try `QProcess::terminate` 
> instead?
> 
> PS: `QProcess` gets unhappy when being deleted while still running (=> 
> runtime warnings).
> 
> PPS: Please read my other advice about quitting LLDB below, too

It's been a while, QProcess::terminate() was an approach I remember preferring 
to avoid possibly because it also led to runtime warnings. I also don't think 
there's much of a difference in what ::kill() and ::terminate() do.
With deleteLater() I rarely if ever see warning messages, probably because it's 
an implicit way of waiting long enough (in the background) for lldb to exit.

> kfunk wrote in backtracegenerator.cpp:144
> This whole logic looks really cumbersome.
> 
> Is there really no way to exit LLDB cleanly after detach?
> 
> http://stackoverflow.com/questions/26267289/how-can-i-exit-lldb-after-running-commands-with-o
>  suggests there is:
> 
>   lldb /bin/ls -o "run" -o "script import os; os._exit(1)"
> 
> I take it not everyone's got Python on they system, but this works for me as 
> well:
> 
>   lldb -p $(pidof kate) -o detach -o quit
> 
> Just put that into the `-o quit` in the lldbrc?

`-o quit` indeed works with newer lldb versions, but not yet with the system 
version on OS X 10.9 .

I suppose I could do a combination of both. The logical way would be 
`\nquit\nscript import os; os._exit(0)` but processing stops after the quit 
command so it'd have to be the opposite. I'd have to test this for a bit.

Not having a python interpreter may not be an issue. I don't know how its 
complete absence is treated but errors in the python expression (loading an 
inexisting module for instance) don't stop processing of subsequent commands.

Can one put comments in the lldbrc file explaining the reason of the surprising 
BatchCommands set?

> kfunk wrote in AppleTerminal:1
> Please explain why this file is needed(?)

It is required to attach lldb in a terminal window on Mac using the native 
terminal application. I don't want to rely on konsole being installed there, in 
part because that application has issues with certain Control keystrokes under 
Qt5 (including Ctrl-C and family).

> kfunk wrote in drkonqibackends.cpp:165
> Why these special conditions? Needs comments.
> 
> Whether to use or not to use lldb on a particular OS X version should be 
> runtime decision, too.

Actually, there can be a runtime decision to use lldb on earlier (10.7-) 
versions but not on later versions. Gdb doesn't work properly from 10.8 
onwards. And to be honest lldb was in such an early stage in 10.7 and earlier 
that you wouldn't want to use the system version there.
Given that gdb still works there it seems unnecessarily complicated to make 
this a runtime decision. Esp. since using gdb means you get access to the 
standard backtrace parser.

I'll put a concise version of this explanation as a comment.

> kfunk wrote in backtraceparserlldb.cpp:26
> Style: Use `const QString &line`
> 
> More of these issues in other lines

I think I preserved the style in the backtracerparser implementation I cloned...

REPOSITORY
  R120 Plasma Workspace

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

To: rjvbb, #plasma_workspaces, kfunk
Cc: kfunk, mart, broulik, kde-mac, plasma-devel, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

Reply via email to