Differential D17301 Diff 46684 src/file/extractor/result.h

Changeset View

Standalone View

View Options

src/file/extractor/result.h

Show All 25 Lines
26#include <KFileMetaData/ExtractionResult> 26​#include <KFileMetaData/ExtractionResult>
27 27​
28#include "document.h" 28​#include "document.h"
29#include "termgenerator.h" 29​#include "termgenerator.h"
30 30​
31namespace Baloo 31​namespace Baloo
32{ 32​{
33 33​
34​/**
35​ * \class Result result.h
36​ *
37​ * \brief The result class is where all the data extracted by
38​ * the KFileMetaData extractors is saved to a Baloo::Document.
brunsUnsubmitted
Done

Remove the last part of the sentence - ... is saved to.. Everything else is an implementation detail.

bruns: Remove the last part of the sentence - `... is saved to.`. Everything else is an implementation…
39​ * It implements the KFileMetaData::ExtractionResult interface.
40​ * The resulting Baloo::Document can then be pushed into the
brunsUnsubmitted
Done

`The results can be retrieved as a Baloo::Document using \c document() and
stored in the database.`

bruns: `The results can be retrieved as a Baloo::Document using \c document() and stored in the…
41​ * database.
42​ *
43​ */
34class Result : public KFileMetaData::ExtractionResult 44​class Result : public KFileMetaData::ExtractionResult
35{ 45​{
36public: 46​public:
37 Result(); 47​ Result();
38 Result(const QString& url, const QString& mimetype, const Flags& flags = ExtractEverything); 48​ Result(const QString& url, const QString& mimetype, const Flags& flags = ExtractEverything);
39 49​
50​ /**
51​ * Overloaded by function from KFileMetaData::ExtractionResult.
brunsUnsubmitted
Done

This is no overload, but an override/reimplementation.

bruns: This is no overload, but an override/reimplementation.
52​ * the plugins when they wish to
53​ * add a key value pair which should be indexed. This function may be
54​ * called multiple times for the same key.
brunsUnsubmitted
Done

I think a \sa KFileMetaData::Property is sufficient here. Also, its not a property name, that would be something like a QString.

bruns: I think a `\sa KFileMetaData::Property` is sufficient here. Also, its not a property //name//…
55​ *
56​ * \p property This specifies a property name. It should be one of the
yurchorUnsubmitted
Done

This should be \param

https://www.stack.nl/~dimitri/doxygen/manual/commands.html#cmdp

https://www.stack.nl/~dimitri/doxygen/manual/commands.html#cmdparam

yurchor: This should be `\param` https://www.stack.nl/~dimitri/doxygen/manual/commands.html#cmdp https…
57​ * properties from the list of properties from KFileMetaData.
58​ *
59​ * \p value The value of the property
yurchorUnsubmitted
Done

Same here.

yurchor: Same here.
60​ */
40 void add(KFileMetaData::Property::Property property, const QVariant& value) override; 61​ void add(KFileMetaData::Property::Property property, const QVariant& value) override;
62​
63​ /**
64​ * Appends plain text to the document. This generally corresponds
65​ * to the text content in a file.
66​ *
67​ * \p text The text that will be appended to the document
yurchorUnsubmitted
Done

Same here.

yurchor: Same here.
68​ */
41 void append(const QString& text) override; 69​ void append(const QString& text) override;
70​
71​ /**
72​ * A type is a higher level classification of the file, e.g.
brunsUnsubmitted
Not Done

Qt uses "Reimplemented from KFileMetaData::ExtractionResult::addtype()`, see e.g. http://doc.qt.io/qt-5/qfile.html#fileName

Overriden has a typo, "Overridden"

But I think you should remove the (docstring) comment, it should automatically pick up the description from the interface class.

bruns: Qt uses "Reimplemented from KFileMetaData::ExtractionResult::addtype()`, see e.g. http://doc.qt.
73​ * a document or an video file.
74​ */
42 void addType(KFileMetaData::Type::Type type) override; 75​ void addType(KFileMetaData::Type::Type type) override;
43 76​
77​ /**
78​ * Sets the Baloo document to which the results from the extractors
79​ * will be saved.
80​ * \p doc The Baloo::Document
yurchorUnsubmitted
Done

Same here.

yurchor: Same here.
81​ */
44 void setDocument(const Baloo::Document& doc); 82​ void setDocument(const Baloo::Document& doc);
brunsUnsubmitted
Done

The use case is missing here.
Can be used to add extraction results to an existing Baloo::Document. Has to be called before passing the Result to KFileMetaData::Extractor::extract()

bruns: The use case is missing here. `Can be used to add extraction results to an existing Baloo…
45 83​
84​ /**
85​ * Returns a QVariantMap containing all metadata fetched from the extractors.
86​ * The metadata is saved as <property.name(),property.value()>
brunsUnsubmitted
Done

Thats not correct, see implementation of Result::add(...)

QString p = QString::number(propnum)

But as m_map/map() is never used outside Result (only in Result::finish), it is best to remove the getter completely. D17312

bruns: Thats not correct, see implementation of `Result::add(...)` `QString p = QString::number…
87​ */
46 QVariantMap map() const; 88​ QVariantMap map() const;
47 89​
90​ /**
91​ * Returns the Baloo document to which the results from the extractors
92​ * will are saved.
93​ */
48 Baloo::Document& document(); 94​ Baloo::Document& document();
49 95​
50 /** 96​ /**
51 * Applies the finishing touches on the document, and makes 97​ * Applies the finishing touches on the document, and makes
52 * it ready to be pushed into the db 98​ * it ready to be pushed into the db
53 */ 99​ */
54 void finish(); 100​ void finish();
55 101​
56private: 102​private:
103​ /**
104​ * The document that represents the file that is to be indexed.
105​ */
57 Baloo::Document m_doc; 106​ Baloo::Document m_doc;
107​ /**
108​ * Contains the collection of words of the metadata
brunsUnsubmitted
Done

Nitpick - missing full stop, also below.

bruns: Nitpick - missing full stop, also below.
brunsUnsubmitted
Done

The TermGenerator's do not contain any data themselves, but

  • keep/update the position state when adding data
  • add the data to the referenced Baloo::Document when supplied with new data (index*Text())
bruns: The TermGenerator's do not contain any data themselves, but - keep/update the position state…
astippichAuthorUnsubmitted
Not Done

Would be nice if you could add that information to the TermGenerator :)

astippich: Would be nice if you could add that information to the TermGenerator :)
109​ */
58 Baloo::TermGenerator m_termGen; 110​ Baloo::TermGenerator m_termGen;
111​ /**
112​ * Contains the collection of words of plain text
113​ */
59 Baloo::TermGenerator m_termGenForText; 114​ Baloo::TermGenerator m_termGenForText;
60 115​ /**
116​ * Contains all indexed data from the extractors
brunsUnsubmitted
Done

Only properties

bruns: Only properties
117​ */
61 QVariantMap m_map; 118​ QVariantMap m_map;
62}; 119​};
63 120​
64} 121​}
65#endif // EXTRACTIONRESULT_H 122​#endif // EXTRACTIONRESULT_H