[PATCH] kopenwith.cpp bugfix and enhancement

[Willy De la Court]

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