[PATCH] kopenwith.cpp bugfix and enhancement
Willy De la Court
Willy.DelaCourt at pandora.be
Tue Feb 11 15:03:12 GMT 2003
On Tuesday 11 February 2003 15:00, David Faure wrote:
> On Tuesday 11 February 2003 14:45, Willy De la Court wrote:
<SNIP>
> > not.
> >
> > Do you agree with my reasoning?
>
> Oh. Now I see. You broke it :(
>
yeah probably but there is still a bug in there and the .desktop files is
created even if you select "Run in terminal"
> The old code modified m_command to contain the command for running a
> terminal, in case the remember checkbox didn't exist or wasn't checked. In
> such a case there is _NO_ creation of a .desktop file, so the writeEntry()
> code is totally irrelevant. But by modifying the command, and setting
> m_pService to 0, we ensure that the caller will run the full command,
> including the terminal.
>
and i think i found it
if ( m_pService && terminal->isChecked() != m_pService->terminal() )
m_pService = 0L; // It's not exactly this service we're running
------> here you set m_pService to 0L like you said to force a the caller to
run the cammand as is
BUT
if ( m_pService && ( !remember || !remember->isChecked() ) ) {
------> but you only accept the run if m_pService is not 0L
accept();
return;
}
------> so we get here and the .desktop file is created
// if we got here, we can't seem to find a service for what they
// wanted. The other possibility is that they have asked for the
// association to be remembered. Create/update service.
> - - KSimpleConfig conf(QString::fromLatin1("konquerorrc"), true);
> - - conf.setGroup(QString::fromLatin1("Misc Defaults"));
anyway these two lines are wrong the setting is not saved there anymore
KSimpleConfig conf(QString::fromLatin1("kdeglobals"), true);
conf.setGroup(QString::fromLatin1("General"));
> - - m_command = conf.readEntry(QString::fromLatin1("Terminal"),
> QString::fromLatin1("konsole")); - -
> - - m_command += QString::fromLatin1(" -e ");
> - - m_command += edit->url();
>
> > Damn and i tought i did a good job of leaving all the functionality
> > intact. So at what should a look in the future to avoid these pitfalls?
>
> http://developer.kde.org/documentation/library/kdeqt/kde3arch/devel-binaryc
>ompatibility.html
thx i'll read that
>
> > I look forward on the applied patch to see what i could have done better.
>
> Please provide a patch that basically reverts to the above code,
> and adds the new option to m_command when the new checkbox is checked.
>
>
> - --
> David Faure -- faure at kde.org, dfaure at klaralvdalens-datakonsult.se
--
Willy De la Court
More information about the kde-core-devel
mailing list