D8528: Consider DjVu files to be documents
This revision was automatically updated to reflect the committed changes. Closed by commit R293:2e80367435cc: Consider DjVu files to be documents (authored by ngraham). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D8528?vs=21461=21597#toc REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8528?vs=21461=21597 REVISION DETAIL https://phabricator.kde.org/D8528 AFFECTED FILES src/file/basicindexingjob.cpp To: ngraham, vhanda, #frameworks, rkflx Cc: rkflx, #frameworks
D8528: Consider DjVu files to be documents
rkflx accepted this revision. rkflx added a comment. This revision is now accepted and ready to land. I'd say this can land. Perhaps best to wait until Monday evening, so Frameworks people not spending their weekend at the computer have a chance to weigh in? REPOSITORY R293 Baloo BRANCH djvu_files_documents_369195 REVISION DETAIL https://phabricator.kde.org/D8528 To: ngraham, vhanda, #frameworks, rkflx Cc: rkflx, #frameworks
D8528: Consider DjVu files to be documents
vhanda added a comment. Since I was added as a reviewer, I thought I'll comment. I am not currently maintaining Baloo or using it, so I don't want to really discuss the specifics. Though from a technical point of view this change will work. If nobody has any objections, I would say go for it. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D8528 To: ngraham, vhanda, #frameworks, rkflx Cc: rkflx, #frameworks
D8528: Consider DjVu files to be documents
ngraham added a reviewer: rkflx. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D8528 To: ngraham, vhanda, #frameworks, rkflx Cc: rkflx, #frameworks
D8528: Consider DjVu files to be documents
ngraham marked 4 inline comments as done. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D8528 To: ngraham, vhanda, #frameworks Cc: rkflx, #frameworks
D8528: Consider DjVu files to be documents
ngraham updated this revision to Diff 21461. ngraham added a comment. Only mark multipage DjVu files as documents, and remove crufty old compatibility mimetype that's probably not relevant at all REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8528?vs=21459=21461 BRANCH djvu_files_documents_369195 REVISION DETAIL https://phabricator.kde.org/D8528 AFFECTED FILES src/file/basicindexingjob.cpp To: ngraham, vhanda, #frameworks Cc: rkflx, #frameworks
D8528: Consider DjVu files to be documents
rkflx added inline comments. INLINE COMMENTS > rkflx wrote in basicindexingjob.cpp:219 > Looking at `/usr/share/mime/image`, I see: > > - `vnd.djvu.xml`: "DjVu image" > - `vnd.djvu+multipage.xml`: "DjVu document" > > …and that's also what Dolphin shows. Probably best to add `+multipage` here? Sorry, with "add `+multipage`" I meant to add exactly this to the existing line, not to add an additional line. This way, "image" DjVu files are still classified as images by baloo, just as in Dolphin. This might be inconvenient in some cases, but that's how the mimetype standard defines it. > ngraham wrote in basicindexingjob.cpp:220 > https://www.cuminas.jp/docs/djvuplugin/en_us/Content/MIME%20Types.htm said > that it was an older one, so I figured I'd add that one for maximum > compatibility. I can remove it if you want. Latest news from djvu.org is from 2013, "older versions //of the DjVu Browser Plug-in//" are probably even more prehistoric and won't run in upcoming browsers anyway. I don't have this on Tumbleweed, if your Kubuntu does not have it I would tend to removal. Also I believe this is just for opening files in the browser and not relevant to indexing at all. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D8528 To: ngraham, vhanda, #frameworks Cc: rkflx, #frameworks
D8528: Consider DjVu files to be documents
ngraham edited the test plan for this revision. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D8528 To: ngraham, vhanda, #frameworks Cc: rkflx, #frameworks
D8528: Consider DjVu files to be documents
ngraham updated this revision to Diff 21459. ngraham marked an inline comment as done. ngraham added a comment. Actually we don't need the xml extension here REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8528?vs=21458=21459 BRANCH djvu_files_documents_369195 REVISION DETAIL https://phabricator.kde.org/D8528 AFFECTED FILES src/file/basicindexingjob.cpp To: ngraham, vhanda, #frameworks Cc: rkflx, #frameworks
D8528: Consider DjVu files to be documents
ngraham added inline comments. INLINE COMMENTS > rkflx wrote in basicindexingjob.cpp:220 > Is `x-djvu` a thing? Not sure, that's why I'm asking. https://www.cuminas.jp/docs/djvuplugin/en_us/Content/MIME%20Types.htm said that it was an older one, so I figured I'd add that one for maximum compatibility. I can remove it if you want. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D8528 To: ngraham, vhanda, #frameworks Cc: rkflx, #frameworks
D8528: Consider DjVu files to be documents
ngraham updated this revision to Diff 21458. ngraham added a comment. Adjusting MIME types to match what's in /usr/share/mime/images REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8528?vs=21455=21458 BRANCH djvu_files_documents_369195 REVISION DETAIL https://phabricator.kde.org/D8528 AFFECTED FILES src/file/basicindexingjob.cpp To: ngraham, vhanda, #frameworks Cc: rkflx, #frameworks
D8528: Consider DjVu files to be documents
rkflx added a comment.
General sentiment makes sense, most DjVu documents I've seen were more like
books. However, see inline comment. (Someone with actual mimetype or baloo
knowledge should approve, though.)
> didn't crash baloo with basic usage
If you don't have a DjVu file, that's not "basic usage", but "no usage",
making it quite pointless to mention the absence of crashes :)
Anyway, see last section here for some example files:
http://www.djvu.org/resources
I guess you need to edit your test plan again…
INLINE COMMENTS
> basicindexingjob.cpp:219
> {"text/markdown", Type::Document},
> +{"image/vnd.djvu", Type::Document},
> +{"image/x-djvu", Type::Document},
Looking at `/usr/share/mime/image`, I see:
- `vnd.djvu.xml`: "DjVu image"
- `vnd.djvu+multipage.xml`: "DjVu document"
…and that's also what Dolphin shows. Probably best to add `+multipage` here?
> basicindexingjob.cpp:220
> +{"image/vnd.djvu", Type::Document},
> +{"image/x-djvu", Type::Document},
> {"application/x-lyx", Type::Document}
Is `x-djvu` a thing? Not sure, that's why I'm asking.
REPOSITORY
R293 Baloo
REVISION DETAIL
https://phabricator.kde.org/D8528
To: ngraham, vhanda, #frameworks
Cc: rkflx, #frameworks
D8528: Consider DjVu files to be documents
ngraham edited the summary of this revision. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D8528 To: ngraham, vhanda, #frameworks Cc: #frameworks
D8528: Consider DjVu files to be documents
ngraham added reviewers: vhanda, Frameworks. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D8528 To: ngraham, vhanda, #frameworks Cc: #frameworks
D8528: Consider DjVu files to be documents
ngraham created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY BUG: 369195 TEST PLAN Tested in KDE Neon. Compiled and deployed fine; didn't crash baloo with basic usage. Unable to test beyond that since I don't have any DjVu files and wasn't able to locale any online for direct download or find an online PDF-to-DjVu converter that worked REPOSITORY R293 Baloo BRANCH djvu_files_documents_369195 REVISION DETAIL https://phabricator.kde.org/D8528 AFFECTED FILES src/file/basicindexingjob.cpp To: ngraham Cc: #frameworks
