[Development] QHelpEngineCore::documentsForIdentifier

[Jaroslaw Kobus]

Hi Martin,

sorry for not answering earlier.

I've just prepared a patch that should fix it. The assumption before was that at some point the default for using new filter engine will change to true. However, since it didn't happen so far, I'm providing a full fix for old filtering system, too.

The patch is here: https://codereview.qt-project.org/c/qt/qttools/+/312105

Could you please confirm it fixes your issue?

Thanks and best regards

Jarek

________________________________________
From: Development <development-bounces at qt-project.org> on behalf of Martin Koller <kollix at aon.at>
Sent: Friday, August 28, 2020 2:09 PM
To: Edward Welbourne; development at qt-project.org; Samuel Gaist
Subject: Re: [Development] QHelpEngineCore::documentsForIdentifier

On Freitag, 28. August 2020 08:38:45 CEST Samuel Gaist wrote:
> Hi,
>
> > On 27 Aug 2020, at 19:07, Edward Welbourne <edward.welbourne at qt.io> wrote:
> >
> > Martin Koller (14 August 2020 17:06) wrote:
> >
> >> I found myself getting an empty list when using
> >> QHelpEngineCore::documentsForIdentifier(id) but getting 1 element when
> >> using the deprecated QHelpEngineCore::linksForIdentifier(id).
> >
> > Definitely sounds like a bug, although I find no
> > QHelpEngineCore::linksForIdentifier() - did you mean
> > QHelpCollectionHandler::linksForIdentifier() ?
> >
> >> Checking the code I stumbled over something which I believe is a bug:
> >>
> >> QHelpEngineCore::documentsForIdentifier(const QString &id, const
> >> QString &filterName) does
> >>
> >> if (!d->setup() || !d->usesFilterEngine)
> >>        return QList<QHelpLink>();
> >>
> >> so it checks if the new filter engine is active, but I think this is
> >> not needed here, since in this code, the filter engine is not used at
> >> all, since the filterName is already given as argument.
> >
> > Sounds plausible.
> > Also, QHelpEngineCore::documentsForIdentifier(const QString &id) calls
> > documentsForIdentifier(const QString &id, const QString &filterName)
> > passing a filterName that it determines based on d->usesFilterEngine,
> > which rather suggests it thinks d->usesFilterEngine isn't a prerequisite
> > of getting any entries.
> >
> >> Since I ported older code to the new api but did not call
> >> setUsesFilterEngine(true) (which I believe should not be needed when I
> >> don't work with filters), this check was hit and I get no results.
> >>
> >> Am I right ?
> >
> > I don't know this code at all but it sounds like reasonable grounds to
> > submit a patch to Gerrit and ask for review by folk implicated in the
> > review that added this code (this March):
> > https://codereview.qt-project.org/c/qt/qttools/+/291144
> >
> >     Eddy.
> > _______________________________________________
> > Development mailing list
> > Development at qt-project.org
> > https://lists.qt-project.org/listinfo/development
>
> You need to enable the new filter engine using:
>
> https://doc.qt.io/qt-5/qhelpenginecore.html#setUsesFilterEngine
>
> The documentation has been modified for the next release to make it clearer.

You mean as a workaround ? Yes, this is what I did - as a workaround.

But my question was about the - in my view - wrong code,
where Edward also seems to share my opinion.
I assume I should not be forced to us a filter engine when I already can
pass a filter to this method (and to be honest I have no idea what the filterEngine
does and have no need for filtering)

--
Best regards/Schöne Grüße

Martin
A: Because it breaks the logical sequence of discussion
Q: Why is top posting bad?

()  ascii ribbon campaign - against html e-mail
/\                        - against proprietary attachments

Frühstück, Geschenkideen, Accessoires, Kulinarisches: www.lillehus.at


_______________________________________________
Development mailing list
Development at qt-project.org
https://lists.qt-project.org/listinfo/development