Re: Review Request 129720: [ExtractorCollection] Use mimetype inheritance to return plugins

[Anthony Fieroni]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129720/
---

(Updated Feb. 5, 2017, 4:06 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Changes
---

Submitted with commit 7c7e985a4678fef5f5d0dd8faa9b9cb42e3844b4 by Anthony 
Fieroni to branch master.


Repository: kfilemetadata


Description
---

startsWith is a weak precondition + guard for duplicate plugin


Diffs
-

  src/extractorcollection.cpp d0fbbea 

Diff: https://git.reviewboard.kde.org/r/129720/diff/


Testing
---


Thanks,

Anthony Fieroni

Re: Review Request 129720: [ExtractorCollection] Use mimetype inheritance to return plugins

[Anthony Fieroni]

> On Jan. 19, 2017, 5:36 p.m., Stefan Brüns wrote:
> > src/extractorcollection.cpp, line 155
> > 
> >
> > This is broken:
> > 
> > if (plugins.isEmpty()) {
> >   ...
> >   for (; it != d->m_extractors.constEnd(); it++) {
> >  if (plugins.contains( some_value )
> > 
> > The "plugins.contains(...)" is always false.

not always, line 150 appending to list, this guard to protect duplicate 
extractor


- Anthony


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129720/#review102096
---


On Dec. 29, 2016, 9:17 a.m., Anthony Fieroni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129720/
> ---
> 
> (Updated Dec. 29, 2016, 9:17 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kfilemetadata
> 
> 
> Description
> ---
> 
> startsWith is a weak precondition + guard for duplicate plugin
> 
> 
> Diffs
> -
> 
>   src/extractorcollection.cpp d0fbbea 
> 
> Diff: https://git.reviewboard.kde.org/r/129720/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

Re: Review Request 129720: [ExtractorCollection] Use mimetype inheritance to return plugins

[Stefan Brüns]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129720/#review102096
---




src/extractorcollection.cpp (line 155)


This is broken:

if (plugins.isEmpty()) {
  ...
  for (; it != d->m_extractors.constEnd(); it++) {
 if (plugins.contains( some_value )

The "plugins.contains(...)" is always false.


- Stefan Brüns


On Dec. 29, 2016, 7:17 a.m., Anthony Fieroni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129720/
> ---
> 
> (Updated Dec. 29, 2016, 7:17 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kfilemetadata
> 
> 
> Description
> ---
> 
> startsWith is a weak precondition + guard for duplicate plugin
> 
> 
> Diffs
> -
> 
>   src/extractorcollection.cpp d0fbbea 
> 
> Diff: https://git.reviewboard.kde.org/r/129720/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

Re: Review Request 129720: [ExtractorCollection] Use mimetype inheritance to return plugins

[Anthony Fieroni]

> On Ян. 2, 2017, 11:42 преди обяд, David Faure wrote:
> > src/extractorcollection.cpp, line 155
> > 
> >
> > Shouldn't this be the other way around?
> > 
> > You're looking for an extractor for type == 
> > application/vnd.oasis.opendocument.text.
> > You find an extractor for it.key() == application/zip.
> > That's not good, it won't know anything about OpenDocument stuff.
> > 
> > So you want db.mimeTypeForName(it.value()).inherits(mimetype), no?
> > 
> > KIO::PreviewJob iterates over type.allAncestors() instead, but now that 
> > I think about it, that seems overkill and doesn't do more than the 
> > suggested line above.

That was desired effect, i.e. we try to find more *abstract* parser. Does we 
can find any parser to 
db.mimeTypeForName(it.key()).inherits("application/vnd.oasis.opendocument.text")
 ? I don't think we can find someone or i'm in wrong :)


- Anthony


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129720/#review101716
---


On Дек. 29, 2016, 9:17 преди обяд, Anthony Fieroni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129720/
> ---
> 
> (Updated Дек. 29, 2016, 9:17 преди обяд)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kfilemetadata
> 
> 
> Description
> ---
> 
> startsWith is a weak precondition + guard for duplicate plugin
> 
> 
> Diffs
> -
> 
>   src/extractorcollection.cpp d0fbbea 
> 
> Diff: https://git.reviewboard.kde.org/r/129720/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

Re: Review Request 129720: [ExtractorCollection] Use mimetype inheritance to return plugins

[David Faure]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129720/#review101716
---




src/extractorcollection.cpp (line 155)


Shouldn't this be the other way around?

You're looking for an extractor for type == 
application/vnd.oasis.opendocument.text.
You find an extractor for it.key() == application/zip.
That's not good, it won't know anything about OpenDocument stuff.

So you want db.mimeTypeForName(it.value()).inherits(mimetype), no?

KIO::PreviewJob iterates over type.allAncestors() instead, but now that I 
think about it, that seems overkill and doesn't do more than the suggested line 
above.


- David Faure


On Dec. 29, 2016, 7:17 a.m., Anthony Fieroni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129720/
> ---
> 
> (Updated Dec. 29, 2016, 7:17 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kfilemetadata
> 
> 
> Description
> ---
> 
> startsWith is a weak precondition + guard for duplicate plugin
> 
> 
> Diffs
> -
> 
>   src/extractorcollection.cpp d0fbbea 
> 
> Diff: https://git.reviewboard.kde.org/r/129720/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

Re: Review Request 129720: [ExtractorCollection] Use mimetype inheritance to return plugins

[Anthony Fieroni]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129720/#review101693
---



ping

- Anthony Fieroni


On Дек. 29, 2016, 9:17 преди обяд, Anthony Fieroni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129720/
> ---
> 
> (Updated Дек. 29, 2016, 9:17 преди обяд)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kfilemetadata
> 
> 
> Description
> ---
> 
> startsWith is a weak precondition + guard for duplicate plugin
> 
> 
> Diffs
> -
> 
>   src/extractorcollection.cpp d0fbbea 
> 
> Diff: https://git.reviewboard.kde.org/r/129720/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>