D18698: [baloo/KInotify] Notify if folder was moved from unwatched place

poboiko updated this revision to Diff 51062. poboiko edited the summary of this revision. poboiko added a comment. Added recursive iteration over all contents for `Create` event as well REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18698?vs=50967=51062

D18688: Check Exiv2::ValueType::typeId before converting it to rational

This revision was automatically updated to reflect the committed changes. Closed by commit R286:6e449d44bb5d: Check Exiv2::ValueType::typeId before converting it to rational (authored by poboiko). REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE

D18688: Check Exiv2::ValueType::typeId before converting it to rational

poboiko created this revision. poboiko added reviewers: Baloo, Frameworks, astippich. Herald added projects: Frameworks, Baloo. poboiko requested review of this revision. REVISION SUMMARY If one has a malformed image with GPSAltitude key present, but is not rational, an attempt to convert it

D18664: Baloo engine: treat every non-success code as a failure

poboiko added a comment. Nice! I like it, it's definitely much better than `Q_ASSERT_X` macros that are just silently ignored in non-debug builds. Have some nitpicks though: 1. Are we actually sure this is gonna fix all those crashes? Otherwise I would suggest using CCBUG instead

D18698: [baloo/KInotify] Notify if folder was moved from unwatched place

poboiko created this revision. poboiko added reviewers: Baloo, Frameworks. Herald added projects: Frameworks, Baloo. poboiko requested review of this revision. REVISION SUMMARY If a folder was moved from an unwatched place, `KInotify` will receive an `EventMoveTo` event, which doesn't have

D18698: [baloo/KInotify] Notify if folder was moved from unwatched place

poboiko marked 2 inline comments as done. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D18698 To: poboiko, #baloo, #frameworks, ngraham, bruns Cc: bruns, ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, abrahams

D18698: [baloo/KInotify] Notify if folder was moved from unwatched place

poboiko added a comment. > I am not sure if I understood your description correctly, but I am quite sure this race condition does not exist - the files/folders inside the moved folder are not created/moved one by one, but the containing folder ist just "renamed" - it is unlinked from the

D18698: [baloo/KInotify] Notify if folder was moved from unwatched place

poboiko marked an inline comment as done. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D18698 To: poboiko, #baloo, #frameworks, ngraham, bruns Cc: bruns, ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, abrahams

D18698: [baloo/KInotify] Notify if folder was moved from unwatched place

poboiko updated this revision to Diff 50932. poboiko added a comment. Added code to work with first entry that pops from FilteredDirIterator (that is the directory itself) Test still works; but we should emit `created()` signal for it as well. REPOSITORY R293 Baloo CHANGES SINCE LAST

D18698: [baloo/KInotify] Notify if folder was moved from unwatched place

poboiko updated this revision to Diff 50832. poboiko edited the summary of this revision. poboiko added a comment. Explained the race condition in summary, expanded test to check if watches were installed correctly. I've encountered some pretty weird issue: the watch for the moved folder

D18698: [baloo/KInotify] Notify if folder was moved from unwatched place

poboiko updated this revision to Diff 50967. poboiko added a comment. Cosmetics REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18698?vs=50932=50967 BRANCH add-watch-moved (branched from master) REVISION DETAIL https://phabricator.kde.org/D18698

D21576: [UnindexedFileIndexer] Do not try to add nonexistant file to index

poboiko accepted this revision. This revision is now accepted and ready to land. REPOSITORY R293 Baloo BRANCH submit_unindexed REVISION DETAIL https://phabricator.kde.org/D21576 To: bruns, #baloo, ngraham, astippich, poboiko Cc: kde-frameworks-devel, LeGast00n, domson, ashaposhnikov,

D21579: [FilteredDirIterator] Avoid RegExp overhead for exact matches

poboiko added inline comments. INLINE COMMENTS > regexpcache.cpp:60 > +} > f.replace(QLatin1Char('.'), QStringLiteral("\\.")); > f.replace(QLatin1Char('?'), QLatin1Char('.')); A quick note for future. There is `QRegularExpression::wildcardToRegularExpression()`, which

D21577: [UnindexedFileIndexer] Skip filetime checks for new files

poboiko added a comment. I didn't know `QFileInfo` fetches information on demand (and caches it). That is the reason for this change, right? I think it would be nice to elaborate on that in summary, or maybe as a brief comment in the code. INLINE COMMENTS > unindexedfileiterator.cpp:109

D21578: [UnindexedFileIterator] Delay mimetype determination until it is needed

poboiko accepted this revision. This revision is now accepted and ready to land. REPOSITORY R293 Baloo BRANCH submit_unindexed REVISION DETAIL https://phabricator.kde.org/D21578 To: bruns, #baloo, ngraham, astippich, poboiko Cc: kde-frameworks-devel, LeGast00n, domson, ashaposhnikov,

D21509: [baloo_file] Index renamed folders inside UnindexedFileIndexer

poboiko updated this revision to Diff 59071. poboiko added a comment. Fixed comment with directory structure REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21509?vs=59070=59071 BRANCH unindexed-renamed REVISION DETAIL https://phabricator.kde.org/D21509

D21509: [baloo_file] Index renamed folders inside UnindexedFileIndexer

poboiko updated this revision to Diff 59068. poboiko added a comment. Split test to three separate test functions, which cover different test cases. Renamed folders to something more meaningful; put all the names as static consts in the very beginning, for further reuse inside test

D21509: [baloo_file] Index renamed folders inside UnindexedFileIndexer

poboiko updated this revision to Diff 59070. poboiko marked 3 inline comments as done. poboiko added a comment. Moved `m_nameChanged` check inside separate block REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21509?vs=59068=59070 BRANCH unindexed-renamed

D21509: [baloo_file] Index renamed folders inside UnindexedFileIndexer

poboiko added inline comments. INLINE COMMENTS > bruns wrote in unindexedfileiterator.cpp:126 > This whole block belongs into a separate block, executed `if > (m_cTimeChanged)`. Did you mean `m_mTimeChanged`? REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D21509 To:

D21509: [baloo_file] Index renamed folders inside UnindexedFileIndexer

poboiko created this revision. poboiko added reviewers: Frameworks, Baloo, bruns. Herald added projects: Frameworks, Baloo. Herald added a subscriber: kde-frameworks-devel. poboiko requested review of this revision. REVISION SUMMARY If some folder was renamed while `baloo_file` was not running,

D21440: Start baloo_file later

poboiko added a comment. In D21440#471144 , @broulik wrote: > The idle tracking is only in the extractor process, not in baloo_file itself. On startup it runs the `UnindexedFileIndexer` and iterates all the folders looking for files to

D21509: [baloo_file] Index renamed folders inside UnindexedFileIndexer

poboiko updated this revision to Diff 59099. poboiko marked 5 inline comments as done. poboiko added a comment. Use single temp dir Fixed `mTime -> cTime` REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21509?vs=59071=59099 BRANCH unindexed-renamed

D21509: [baloo_file] Index renamed folders inside UnindexedFileIndexer

poboiko added inline comments. INLINE COMMENTS > bruns wrote in unindexedfileiteratortest.cpp:100 > Make this plain members, not pointers. > Also, one temporary dir is enough, you can put the db and the test tree > side-by-side. I wanted both DB and directory tree to be recreated from scratch

D21427: Always skip trailing slashes in FilderedDirIterator

poboiko edited the summary of this revision. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D21427 To: poboiko, #frameworks, #baloo, bruns Cc: kde-frameworks-devel, gennad, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams

D21427: Always skip trailing slashes in FilderedDirIterator

poboiko created this revision. poboiko added reviewers: Frameworks, Baloo, bruns. Herald added projects: Frameworks, Baloo. Herald added a subscriber: kde-frameworks-devel. poboiko requested review of this revision. REVISION SUMMARY I've encountered weird regression. Here is the description:

D21427: Always skip trailing slashes in FilderedDirIterator

poboiko edited the summary of this revision. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D21427 To: poboiko, #frameworks, #baloo, bruns Cc: kde-frameworks-devel, gennad, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams

D21509: [UnIndexedFileIteratorTest] Add tests

poboiko retitled this revision from "[baloo_file] Index renamed folders inside UnindexedFileIndexer" to "[UnIndexedFileIteratorTest] Add tests". poboiko edited the summary of this revision. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D21509 To: poboiko, #frameworks,

D21509: [baloo_file] Index renamed folders inside UnindexedFileIndexer

poboiko updated this revision to Diff 59951. poboiko added a comment. Rebase on master. Since we now reindex folder always when cTime changes, revert `nameChanged()` routine Cover the case when we feed `UnIndexedFileIterator` with path that ends with `/` REPOSITORY R293 Baloo

D21839: [TermGenerator] Use UTF-8 ByteArray for termList

poboiko added a comment. Actually, there is an issue with that code right now, which I wanted to fix, but forgot. The trimming part `finalArr = finalArr.mid(0, maxTermSize);` actually should be performed on `QString` instead of `QByteArray` - unicode symbols inside term can consist of two

D21839: [TermGenerator] Use UTF-8 ByteArray for termList

poboiko added a comment. > As the limit is somewhat arbitrary, maybe we can just limit the QString? I don't think this has any serious side effects. Yep, that's what I've suggested (if I understood you correctly). I guess, we can put the trimming part right to `termList()`, it will

D21706: [ModifiedFileIndexer] Use correct mimetype for folders, delay until needed

poboiko added inline comments. INLINE COMMENTS > modifiedfileindexer.cpp:68 > QFileInfo fileInfo(filePath); > - > -// A folders mtime is updated when a new file is added / removed / > renamed > -// we don't really need to reindex a folder when that happens > -//

D21697: [BasicIndexingJob] Skip lookup of baloo document type for directories

poboiko accepted this revision. This revision is now accepted and ready to land. REPOSITORY R293 Baloo BRANCH basicindexingjob REVISION DETAIL https://phabricator.kde.org/D21697 To: bruns, #baloo, ngraham, astippich, poboiko Cc: kde-frameworks-devel, LeGast00n, domson, ashaposhnikov,

D21672: [PowerStateMonitor] Be conservative when determining power state

poboiko accepted this revision. poboiko added a comment. This revision is now accepted and ready to land. Makes sense to me REPOSITORY R293 Baloo BRANCH scheduler REVISION DETAIL https://phabricator.kde.org/D21672 To: bruns, #baloo, ngraham, astippich, poboiko Cc:

D21703: [Transaction] Replace template for functor with std::function

poboiko added a comment. I thought about it myself. I googled it a bit (i.e. here ) and saw that there might be some quite unwanted runtime overhead because of using `std::function`. It might be negligible (since we're

D21671: [FileIndexScheduler] Stop the indexer when quit() is called via DBus

poboiko accepted this revision. poboiko added a comment. This revision is now accepted and ready to land. That's a nice catch! REPOSITORY R293 Baloo BRANCH scheduler REVISION DETAIL https://phabricator.kde.org/D21671 To: bruns, #baloo, ngraham, astippich, poboiko Cc:

D21673: [FileIndexScheduler] Ensure indexer is not run in suspended state

poboiko accepted this revision. poboiko added a comment. This revision is now accepted and ready to land. Apart from small nitpick, I think it's fine. INLINE COMMENTS > fileindexscheduler.h:131 > bool m_isGoingIdle; > +bool m_isSuspended = false; > }; I think it's inconvenient to

D21698: Move invariant IndexingLevel out of the loop

poboiko accepted this revision. This revision is now accepted and ready to land. REPOSITORY R293 Baloo BRANCH invariants REVISION DETAIL https://phabricator.kde.org/D21698 To: bruns, #baloo, ngraham, astippich, poboiko Cc: kde-frameworks-devel, LeGast00n, domson, ashaposhnikov, michaelh,

D21704: [FirstRunIndexer] Use correct mimetype for folders

poboiko accepted this revision. This revision is now accepted and ready to land. REPOSITORY R293 Baloo BRANCH cleanup REVISION DETAIL https://phabricator.kde.org/D21704 To: bruns, #baloo, ngraham, astippich, poboiko Cc: kde-frameworks-devel, LeGast00n, domson, ashaposhnikov, michaelh,

D21705: [NewFileIndexer] Use correct mimetype for folders, check excludeFolders

poboiko accepted this revision. poboiko added a comment. This revision is now accepted and ready to land. Can we also cover this case with tests? REPOSITORY R293 Baloo BRANCH cleanup REVISION DETAIL https://phabricator.kde.org/D21705 To: bruns, #baloo, ngraham, astippich, poboiko Cc:

D21427: Always skip trailing slashes in FilderedDirIterator

poboiko added a comment. Ping. I've found a way to reproduce a related issue: $ mkdir ~/test $ balooctl config add includeFolders ~/test $ balooctl stop $ balooctl start This prints an error: replace called with invalid arguments, docId: url: "~/test/"

D21427: Always skip trailing slashes in FilderedDirIterator

poboiko added a comment. Ping! Apparently, it does fix bug 409257, which is pretty serious one (db corruption, after all). REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D21427 To: poboiko, #frameworks, #baloo, bruns, ngraham Cc: kde-frameworks-devel, LeGast00n,

D22392: [balooctl/baloo_file_extractor] Consolidate code that performs actual indexing

poboiko created this revision. poboiko added reviewers: Baloo, bruns, ngraham. Herald added projects: Frameworks, Baloo. poboiko requested review of this revision. REVISION SUMMARY The file contents can be indexed in two ways: either by `baloo_file_extractor` after being added to

D21427: Always skip trailing slashes in FilderedDirIterator

poboiko edited the summary of this revision. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D21427 To: poboiko, #frameworks, #baloo, bruns, ngraham Cc: kde-frameworks-devel, LeGast00n, fbampaloukas, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns,

D21427: Always skip trailing slashes in FilderedDirIterator

poboiko added a comment. In D21427#494174 , @bruns wrote: > In D21427#494010 , @poboiko wrote: > > > Ping! > > > > Apparently, it does fix bug 409257, which is pretty serious one (db

D21427: Always skip trailing slashes in FilderedDirIterator

poboiko edited the summary of this revision. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D21427 To: poboiko, #frameworks, #baloo, bruns, ngraham Cc: kde-frameworks-devel, LeGast00n, fbampaloukas, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns,

D21427: Always skip trailing slashes in FilderedDirIterator

poboiko added a reviewer: ngraham. poboiko added a comment. Not entirely sure, but bug 409257 might be caused by that (at least in my case, it looked the same). REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D21427 To:

D23200: Fixes a crash in Peruse triggered by baloo

poboiko added a comment. AFAIK `canonicalFilePath()` cannot return path without slash. The result is either empty (if path does not exist; this is covered by first `if`), or a proper path. The second check is redundant. REPOSITORY R293 Baloo BRANCH fixCrashWithEmptyFolder REVISION

D23787: [baloo_file_extractor] Improve handling of large plain-text files

poboiko created this revision. poboiko added reviewers: Baloo, bruns, ngraham. Herald added projects: Frameworks, Baloo. poboiko requested review of this revision. REVISION SUMMARY First of all, not all plain text-based mimetypes starts with `text/`: i.e. `application/sql` for SQL dumps

D22942: [kded/baloosearchmodule] Remove useless qDebug messages

poboiko created this revision. poboiko added reviewers: Baloo, bruns, ngraham. Herald added projects: Frameworks, Baloo. poboiko requested review of this revision. REVISION SUMMARY Those messages are not informative anyway; they are categorized as "kdeinit5"; and they tend to spam logs during

D23008: [baloo_file_extractor] Be more resistant to multiple QSocketNotifier events

poboiko created this revision. poboiko added reviewers: Baloo, bruns, ngraham. Herald added projects: Frameworks, Baloo. poboiko requested review of this revision. REVISION SUMMARY When a new batch arrives at stdin (as notified by QSocketNotifier), extractor creates a new transaction. If

D22942: [kded/baloosearchmodule] Remove useless qDebug messages

poboiko closed this revision. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D22942 To: poboiko, #baloo, bruns, ngraham Cc: kde-frameworks-devel, LeGast00n, fbampaloukas, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams

D23008: [baloo_file_extractor] Be more resistant to multiple QSocketNotifier events

poboiko added inline comments. INLINE COMMENTS > anthonyfieroni wrote in app.cpp:76-81 > processNextFile should be called till m_ids ends, why it does not happen, > probably that's why @bruns reject your patch Well, if that happens, something went wrong, and we have to do something. As far as

D23008: [baloo_file_extractor] Be more resistant to multiple QSocketNotifier events

poboiko added a comment. In D23008#508782 , @bruns wrote: > @poboiko - is the problem you describe purely theoretical, or did you actually see it happen? > > Multiple triggers of `slotNewInput` are **not** a problem, the code flow reaches

D22502: [FileIndexerConfig] skip invalid entries from included/excludedFolders

poboiko added a comment. In D22502#496812 , @bruns wrote: > The correct fix is to check the returned/calculated ID in the IndexCleaner, otherwise its racy. I think both can be implemented. I had in mind the case when folder was

D22557: [IndexCleaner] ignore non-existent entries inside config

poboiko created this revision. poboiko added reviewers: Baloo, bruns, ngraham. Herald added projects: Frameworks, Baloo. poboiko requested review of this revision. REVISION SUMMARY If a folder was added to include/excludeFolders and then removed, there will be a stale entry in the config, for

D22502: [FileIndexerConfig] skip invalid entries from included/excludedFolders

poboiko edited the summary of this revision. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D22502 To: poboiko, #baloo, bruns, ngraham Cc: kde-frameworks-devel, LeGast00n, sbergeron, fbampaloukas, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns,

D22502: [FileIndexerConfig] skip invalid entries from included/excludedFolders

poboiko created this revision. poboiko added reviewers: Baloo, bruns, ngraham. Herald added projects: Frameworks, Baloo. poboiko requested review of this revision. REVISION SUMMARY It is possible to create an invalid entry inside includedFolders, for example: by creating some folder, adding

D22557: [IndexCleaner] ignore non-existent entries inside config

poboiko closed this revision. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D22557 To: poboiko, #baloo, bruns, ngraham Cc: kde-frameworks-devel, LeGast00n, sbergeron, fbampaloukas, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams

D22166: [AdvancedQueryParser] Introduce support for phrase queries

poboiko created this revision. poboiko added reviewers: Baloo, ngraham, bruns. Herald added projects: Frameworks, Baloo. poboiko requested review of this revision. REVISION SUMMARY AdvancedQueryParser's lexxer can actually handle double quotes: for query `a "b c" d`, it returns three tokens

D22166: [AdvancedQueryParser] Introduce support for phrase queries

poboiko added a comment. Note that it somewhat duplicates work done inside `QueryParser`. Which is //almost// not used anywhere - most of the parsing is done by `AdvancedQueryParser`. I feel somewhat weird having two separate parsers, as well as two different entities which store

D23787: [baloo_file_extractor] Improve handling of large plain-text files

poboiko added a comment. Ping? REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D23787 To: poboiko, #baloo, bruns, ngraham Cc: davidedmundson, broulik, kde-frameworks-devel, #baloo, hurikhan77, lots0logs, LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh,

D23787: [baloo_file_extractor] Improve handling of large plain-text files

poboiko edited the summary of this revision. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D23787 To: poboiko, #baloo, bruns, ngraham Cc: davidedmundson, broulik, kde-frameworks-devel, #baloo, hurikhan77, lots0logs, LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov,

D23787: [baloo_file_extractor] Improve handling of large plain-text files

poboiko added a comment. @bruns: I've missed D16593: [ExtractorCollection] Use only best matching extractor plugin , and had in mind previous situation where we've matched all extractors based on inheritance. In that case, "Secondly" part indeed does not

D23787: [baloo_file_extractor] Improve handling of large plain-text files

poboiko marked an inline comment as done. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D23787 To: poboiko, #baloo, bruns, ngraham Cc: broulik, kde-frameworks-devel, #baloo, lots0logs, LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun,

D23787: [baloo_file_extractor] Improve handling of large plain-text files

poboiko updated this revision to Diff 66805. poboiko added a comment. Address raised issue: fetch file size only once REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D23787?vs=65646=66805 BRANCH improve-large-text-files (branched from master) REVISION

D23787: [baloo_file_extractor] Improve handling of large plain-text files

poboiko added inline comments. INLINE COMMENTS > bruns wrote in app.cpp:185 > Users will love us for spammig the logs ... I though users might actually want to know if file was excluded (and the reasoning behind it). I can make its severity less, i.e. `qCDebug`. Or you think it should be

D23787: [baloo_file_extractor] Improve handling of large plain-text files

poboiko updated this revision to Diff 67318. poboiko added a comment. Minor optimization: change check order (filesize / extractor property) Also, it should be better check by "Id" instead of "Name" REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE

D23787: [baloo_file_extractor] Improve handling of large plain-text files

poboiko marked an inline comment as done. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D23787 To: poboiko, #baloo, bruns, ngraham Cc: davidedmundson, broulik, kde-frameworks-devel, #baloo, lots0logs, LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh,

D23787: [baloo_file_extractor] Improve handling of large plain-text files

poboiko added a comment. In D23787#537891 , @bruns wrote: > Can you please provide an example which: > > - is currently indexed though it should be skipped due to size > - is skipped after this change Sure. Any mimetype inherited

D28819: [KRichTextEdit] Always treat key press as single modification in undo stack

This revision was automatically updated to reflect the committed changes. Closed by commit R310:1d1eb6feb7f8: [KRichTextEdit] Always treat key press as single modification in undo stack (authored by poboiko). REPOSITORY R310 KTextWidgets CHANGES SINCE LAST UPDATE

D28854: [KRichTextWidget] Add support for headings

poboiko created this revision. poboiko added reviewers: Frameworks, mlaurent, ahmadsamir, dfaure. Herald added a project: Frameworks. poboiko requested review of this revision. REVISION SUMMARY This patch adds support of different headings (essentially, HTML h1..h6 tags). Those might be

D28854: [KRichTextWidget] Add support for headings

poboiko edited the test plan for this revision. REPOSITORY R310 KTextWidgets REVISION DETAIL https://phabricator.kde.org/D28854 To: poboiko, #frameworks, mlaurent, ahmadsamir, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28819: [KRichTextEdit] Always treat key press as single modification in undo stack

poboiko added a comment. @dfaure There seem to be a regression caused by this patch: if I reach the end of current view, and then press Enter so it has to scroll down, it scrolls to the very beginning of the document instead. This is the same issue as in ancient bug 195828

D28854: [KRichTextWidget] Add support for headings

poboiko marked 3 inline comments as done. poboiko added a comment. In D28854#649189 , @abstractdevelop wrote: > Hey, thanks for this @poboiko I used to have to implement this myself, so this will be very useful in my app, O20.Word. ;)

D28854: [KRichTextWidget] Add support for headings

poboiko closed this revision. REPOSITORY R310 KTextWidgets REVISION DETAIL https://phabricator.kde.org/D28854 To: poboiko, #frameworks, mlaurent, ahmadsamir, dfaure Cc: abstractdevelop, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28819: [KRichTextEdit] Always treat key press as single modification in undo stack

poboiko added a comment. In D28819#651007 , @dfaure wrote: > The *ideal* way of proceeding is to submit a Qt bug report with your testcase (after searching for an existing Qt bug report for the same issue), then turn it into a Qt autotest and

D28819: [KRichTextEdit] Always treat key press as single modification in undo stack

poboiko edited the summary of this revision. REPOSITORY R310 KTextWidgets REVISION DETAIL https://phabricator.kde.org/D28819 To: poboiko, #frameworks, mlaurent Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28819: [KRichTextEdit] Always treat key press as single modification in undo stack

poboiko created this revision. poboiko added reviewers: Frameworks, mlaurent. Herald added a project: Frameworks. poboiko requested review of this revision. REVISION SUMMARY If my cursor is on a bullet list, and I press Enter to create a new element, it registers as 5-7 actions in an undo

D28968: [KRichTextEdit] Make sure headings don't mess with undo stack

poboiko added inline comments. INLINE COMMENTS > dfaure wrote in krichtextedit.cpp:572 > Where is the corresponding beginEditBlock()? `joinPreviousEditBlock` in a way acts as `beginEditBlock` (see https://doc.qt.io/qt-5/qtextcursor.html#joinPreviousEditBlock). The intent here was following:

D28967: [KRichTextEditor] Add support for headings

This revision was automatically updated to reflect the committed changes. Closed by commit R263:2f86040cab5b: [KRichTextEditor] Add support for headings (authored by poboiko). REPOSITORY R263 KXmlGui CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28967?vs=80524=80530 REVISION

D28966: [KRichTextEdit] Fix scroll jumping around when horizontal rule is added

This revision was automatically updated to reflect the committed changes. Closed by commit R310:0e14353610fa: [KRichTextEdit] Fix scroll jumping around when horizontal rule is added (authored by poboiko). REPOSITORY R310 KTextWidgets CHANGES SINCE LAST UPDATE

D28967: [KRichTextEditor] Add support for headings

poboiko updated this revision to Diff 80524. poboiko added a comment. Update version REPOSITORY R263 KXmlGui CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28967?vs=80507=80524 BRANCH heading (branched from master) REVISION DETAIL https://phabricator.kde.org/D28967

D28968: [KRichTextEdit] Make sure headings don't mess with undo stack

This revision was automatically updated to reflect the committed changes. Closed by commit R310:7d3a1386bee2: [KRichTextEdit] Make sure headings dont mess with undo stack (authored by poboiko). REPOSITORY R310 KTextWidgets CHANGES SINCE LAST UPDATE

D28964: [KRichTextWidget] Remove ancient workaround and fix regression (commit 1d1eb6f)

poboiko created this revision. poboiko added reviewers: Frameworks, dfaure, ahmadsamir. Herald added a project: Frameworks. poboiko requested review of this revision. REVISION SUMMARY This patch removes the workaround, which (according to comment) comes from Qt4 times. Although I wasn't even

D28966: [KRichTextEdit] Fix scroll jumping around when horizontal rule is added

poboiko created this revision. poboiko added reviewers: Frameworks, dfaure. Herald added a project: Frameworks. poboiko requested review of this revision. REVISION SUMMARY Due to Qt bug 83605, it's not a good idea to `setTextCursor` while the cursor is inside `beginEditBlock / endEditBlock`

D28964: [KRichTextWidget] Remove ancient workaround and fix regression (commit 1d1eb6f)

This revision was automatically updated to reflect the committed changes. Closed by commit R310:0494fc4a2838: [KRichTextWidget] Remove ancient workaround and fix regression (commit 1d1eb6f) (authored by poboiko). REPOSITORY R310 KTextWidgets CHANGES SINCE LAST UPDATE

D28967: [KRichTextEditor] Add support for headings

poboiko created this revision. poboiko added a reviewer: Frameworks. Herald added a project: Frameworks. poboiko requested review of this revision. REVISION SUMMARY Add new actions introduced in D28854: [KRichTextWidget] Add support for headings to the

D28968: [KRichTextEdit] Make sure headings don't mess with undo stack

poboiko created this revision. poboiko added reviewers: Frameworks, dfaure. Herald added a project: Frameworks. poboiko requested review of this revision. REVISION SUMMARY This patch ensures that everything related to headings is undoable with a single `Ctrl+Z`. It also adds tests for the

D29210: [NestedListHelper] Fix indentation of selection, add tests

poboiko created this revision. poboiko added reviewers: Frameworks, dfaure, mlaurent. poboiko requested review of this revision. REVISION SUMMARY When we try to increase / decrease an indentation level of a selection, just change indentation level of each block in the selection, and then

D29208: [NestedListHelper] Improve indentation code

poboiko created this revision. poboiko added reviewers: Frameworks, dfaure, mlaurent. Herald added a project: Frameworks. poboiko requested review of this revision. REVISION SUMMARY The patch includes following improvements: 1. `handleAfterKeyPressEvent` was only used to adjust margins. We

D29210: [NestedListHelper] Fix indentation of selection, add tests

poboiko updated this revision to Diff 81585. poboiko added a comment. Make the code that removes the block from the list more obvious (without `setObjectIndex` magic) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29210?vs=81267=81585 BRANCH change-indent (branched from master)

D29210: [NestedListHelper] Fix indentation of selection, add tests

poboiko added inline comments. INLINE COMMENTS > dfaure wrote in nestedlisthelper.cpp:225 > should the opposite happen? delete the list when decreasing indentation? > > Or is that what the blkFmt.setObjectIndex(-1); is about? > A comment there would be useful... > > To me setObjectIndex(-1)

D29208: [NestedListHelper] Improve indentation code

poboiko added inline comments. INLINE COMMENTS > dfaure wrote in nestedlisthelper.cpp:87 > So the last block cannot be unindented? How come? That's the current block being checked, not the next one. I've just checked to be sure, last block can be unindented :) TBH, I don't really know if it's

D29208: [NestedListHelper] Improve indentation code

poboiko updated this revision to Diff 81738. poboiko added a comment. Improve readability as suggested, also const'ify REPOSITORY R310 KTextWidgets CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29208?vs=81256=81738 BRANCH unused (branched from master) REVISION DETAIL

D29208: [NestedListHelper] Improve indentation code

This revision was automatically updated to reflect the committed changes. Closed by commit R310:18a2371fe394: [NestedListHelper] Improve indentation code (authored by poboiko). REPOSITORY R310 KTextWidgets CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29208?vs=81738=81743 REVISION

D29210: [NestedListHelper] Fix indentation of selection, add tests

This revision was automatically updated to reflect the committed changes. Closed by commit R310:7012493410ae: [NestedListHelper] Fix indentation of selection, add tests (authored by poboiko). Herald added a project: Frameworks. REPOSITORY R310 KTextWidgets CHANGES SINCE LAST UPDATE

D28854: [KRichTextWidget] Add support for headings

poboiko updated this revision to Diff 80202. poboiko added a comment. Added a unit-test testing for everything I could come up with (at least 4 "behavior nuances" from the commit message) Address other feedback as well (const'ify, @since version) REPOSITORY R310 KTextWidgets

D29826: [KMainWindow] Invoke QIcon::setFallbackThemeName (later)

poboiko updated this revision to Diff 83129. poboiko added a comment. Don't override if fallbackThemeName is already set REPOSITORY R263 KXmlGui CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29826?vs=83109=83129 BRANCH icon-load (branched from master) REVISION DETAIL

D29826: [KMainWindow] Invoke QIcon::setFallbackThemeName (later)

poboiko added a comment. Thanks for looking into it! :) In D29826#673543 , @aacid wrote: > I don't think moving this code from KIconThemes to kmainwindow makes sense, what about all the apps that use KIconThemes but no KMainWindow?

D29826: [KMainWindow] Invoke QIcon::setFallbackThemeName (later)

poboiko added a comment. In D29826#673585 , @aacid wrote: > What about discover for example? Good point... REPOSITORY R263 KXmlGui REVISION DETAIL https://phabricator.kde.org/D29826 To: poboiko, aacid, mart, broulik Cc: mart,

<    1   2   3   >