From konsole-devel Sat Dec 03 17:58:05 2016 From: Kurt Hindenburg Date: Sat, 03 Dec 2016 17:58:05 +0000 To: konsole-devel Subject: Re: Review Request 129214: Added possibility to give default dir on Part instantiation Message-Id: <20161203175805.23280.32188 () mimi ! kde ! org> X-MARC-Message: https://marc.info/?l=konsole-devel&m=148078788913206 --===============0730926120262340654== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit > On Nov. 7, 2016, 7:26 p.m., Kurt Hindenburg wrote: > > Thanks - can you answer Martin's question and provide the yakuake code that is calling this? I don't object to the code, I want to double-check what you're doing. > > Sven Fischer wrote: > Hi Kurt, > > I thought I had answered the question, but obviously it doesn't show up here... > > For the code: Please have a look at https://git.reviewboard.kde.org/r/129215/diff/1#index_header, file app/terminal.cpp. The Part is instantiated by > > factory = KPluginLoader(service->library()).factory(); > > and then > > m_part = factory ? (factory->create(parent, defaults)) : 0; > > So the defaults parameter is the one to use - in yakuake there is no direct inclusion of the Part header, which is indeed the idea behind the PluginLoader factory. That's the reason why I'm not able to use createSession. > > Martin Tobias Holmedahl Sandsmark wrote: > Maybe use the existing TerminalInterface::showShellInDir() instead? Or extend TerminalInterface? I just don't like using opaque QVariant maps with arbitrary strings... I tend to agree with Martin. Sven, have you tried using the TerminalInterface? - Kurt ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129214/#review100697 ----------------------------------------------------------- On Oct. 27, 2016, 6:44 a.m., Sven Fischer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129214/ > ----------------------------------------------------------- > > (Updated Oct. 27, 2016, 6:44 a.m.) > > > Review request for Konsole. > > > Repository: konsole > > > Description > ------- > > On konsole KPart instantiation, the QVariantList is evaluated for a > default directory to change to. The Part honors the profile setting "Use > same directory...". If it is not set, the directory is ignored. > > Simplified the argument parsing > > Beautified the source by using C++11 iteration and QVariantMap > > > Diffs > ----- > > src/Part.cpp 7968176f2b977f391b44dc36a9df9597b27aff2d > > Diff: https://git.reviewboard.kde.org/r/129214/diff/ > > > Testing > ------- > > Built a new version of yakuake against this konsolepart.so. Worked perfectly. > > > Thanks, > > Sven Fischer > > --===============0730926120262340654== MIME-Version: 1.0 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 7bit

This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129214/

On November 7th, 2016, 7:26 p.m. UTC, Kurt Hindenburg wrote:

Thanks - can you answer Martin's question and provide the yakuake code that is calling this? I don't object to the code, I want to double-check what you're doing.

On November 7th, 2016, 7:42 p.m. UTC, Sven Fischer wrote:

Hi Kurt,
I thought I had answered the question, but obviously it doesn't show up here...
For the code: Please have a look at https://git.reviewboard.kde.org/r/129215/diff/1#index_header, file app/terminal.cpp. The Part is instantiated by 
factory = KPluginLoader(service->library()).factory(); 
and then
m_part = factory ? (factory->create<KParts::Part>(parent, defaults)) : 0;
So the defaults parameter is the one to use - in yakuake there is no direct inclusion of the Part header, which is indeed the idea behind the PluginLoader factory. That's the reason why I'm not able to use createSession.

On November 9th, 2016, 1:22 p.m. UTC, Martin Tobias Holmedahl Sandsmark wrote:

Maybe use the existing TerminalInterface::showShellInDir() instead? Or extend TerminalInterface? I just don't like using opaque QVariant maps with arbitrary strings...

I tend to agree with Martin. Sven, have you tried using the TerminalInterface?

- Kurt

On October 27th, 2016, 6:44 a.m. UTC, Sven Fischer wrote:

Review request for Konsole.

By Sven Fischer.

Updated Oct. 27, 2016, 6:44 a.m.

Repository: konsole

Description

On konsole KPart instantiation, the QVariantList is evaluated for a
default directory to change to. The Part honors the profile setting "Use
same directory...". If it is not set, the directory is ignored.

Simplified the argument parsing

Beautified the source by using C++11 iteration and QVariantMap

Testing

Built a new version of yakuake against this konsolepart.so. Worked perfectly.

Diffs

src/Part.cpp (7968176f2b977f391b44dc36a9df9597b27aff2d)

--===============0730926120262340654==--