[PATCH] kopenwith.cpp bugfix and enhancement

David Faure dfaure at klaralvdalens-datakonsult.se
Tue Feb 11 15:02:17 GMT 2003


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Tuesday 11 February 2003 16:03, Willy De la Court wrote:
> > 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

Ok, I was wrong. We do create a .desktop file if we can't find one that does
what we want. Seems the caller code relies on a service existing in all cases.

>   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

Yes, but that's fine.

>   // if we got here, we can't seem to find a service for what they
>   // wanted.

Exactly as this comment says: we couldn't find the service (.desktop file), so we create it.
But we don't put the mimetype association into it.

> anyway these two lines are wrong the setting is not saved there anymore
Ah, good catch.

> > - -    m_command = conf.readEntry(QString::fromLatin1("Terminal"),
> > QString::fromLatin1("konsole")); - -
> > - -    m_command += QString::fromLatin1(" -e ");
> > - -    m_command += edit->url();

Please note that m_command is a member variable (as the m_ indicates), and
that it's the one that the caller can use, to find out which command to use.
Therefore the new code like
  m_command = conf.readEntry(QString::fromLatin1("TerminalApplication"), QString::fromLatin1("konsole"));
(unconditionnally) 
is a very bad idea, since it breaks KOpenWithDlg::text().

The reading of that setting in a single place is a good idea, I would do it
in a local QString in slotOK. But the code that sets m_command should
go back to the "if ( in terminal )" case.

- -- 
David Faure -- faure at kde.org, dfaure at klaralvdalens-datakonsult.se
Klarälvdalens Datakonsult AB, Platform-independent software solutions
Contributing to: http://www.konqueror.org/, http://www.koffice.org/
KOffice-1.2.1 is available - http://download.kde.org/stable/koffice-1.2.1/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.7 (GNU/Linux)

iD8DBQE+SRB572KcVAmwbhARAn5EAJ974LykHFGwIb5pvvSEL0hot/2cMgCdF1UZ
mUYDAdHQQLlxv6hX0fJcv6E=
=YPDR
-----END PGP SIGNATURE-----





More information about the kde-core-devel mailing list