- prefer file mode over content-based mime type
- update preview on update of file mode and change of url
- fix crashes when switching views with opened preview
BUG: 398560
Details
Details
- Reviewers
kossebau cullmann - Group Reviewers
Kate - Commits
- R40:3699973443f2: Use file mode in Preview add-on
- start empty kate session
- enable preview add-on
- open previewable document to replace empty default document
- with patch the preview is displayed
- change the file mode to Normal
- preview is cleared
- restore the original file mode
- preview is displayed again
- close file
- preview is cleared
- open a few previewable files and lock one of them, switch tabs
- close the locked file
- previous behaviour with locked previews is maintained
Diff Detail
Diff Detail
- Repository
- R40 Kate
- Branch
- previewmode
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 136 Build 136: arc lint + arc unit
Comment Actions
Thanks for taking up the challenge to fix some of the glitches with the preview plugin. Happy to see that.
Did not yet get around for some real review, but here at least some first quick comments.
| addons/preview/previewwidget.cpp | ||
|---|---|---|
| 117–118 | Unrelated, please separate commit. | |
| 142–143 | One not yet implemented idea has been that the preview would keep/memorize the state (e.g.scroll position, zoom) it is in. So when having multiple views and switching between the views, the preview would switch to the state matching the view. So not doing things on view->document() == m_previewedTextEditorDocument would introduce an assumption which might make implementing that idea harder in the future. | |
| 169 | Does this also work in KDevelop? Could there be any challenge in the order of entries in the mimetypes list read for a mode? | |
Comment Actions
Thanks for taking up the challenge to fix some of the glitches with the preview plugin. Happy to see that.
I actually tried to edit the SDDM readme and had to realize that the mode is detected as some matlab file type instead of markdown. I thought I should find out why that is and why I can't change it.
I realized today that (using the patch) KDevelop crashes when closing the program with an opened preview. m_partView is deleted before the document gets closed and the PreviewWidget does not know about this. Using a QPointer somehow only shifts the problem (or uncovers another?) deeper into KTextEditor::Document. So this I have to fix first.
| addons/preview/previewwidget.cpp | ||
|---|---|---|
| 142–143 | I see, my new approach would be to just keep and check both (m_previewedTextEditorDocument and m_previewedTextEditorView). | |
| 169 | This is how KTextEditor is getting the mode information (therefore the same for KDevelop). There is no public interface to this KTE::DocumentPrivate data however and I didn't want to start a discussion whether it's worth changing the KTE API for that. I think (to be checked), the order in the list is the one which is set in the config file. So if another mimetype should be preferred, the user has to change the configuration. For MD I actually had to add the mime type manually in (Editor Configuration->Open/Save->Modes & Filetypes). | |
Comment Actions
- restore check for active view
- use QPointer for m_partView, which may be deleted by KDevelop automatically
Comment Actions
I realized today that (using the patch) KDevelop crashes when closing the program with an opened preview. m_partView is deleted before the document gets closed and the PreviewWidget does not know about this. Using a QPointer somehow only shifts the problem (or uncovers another?) deeper into KTextEditor::Document. So this I have to fix first.
D12527 and QPointer for m_partView is fixing this.
One thing which remains and which I just can't figure out, is a crash when switching to Patch Review with an opened preview.
Before this patch KDevelop just crashes inside m_xmlGuiFactory->removeClient(m_partView->kPart()) when switching back to normal mode. With this patch it crashes when changing previews after switching back from Patch Review. Somehow one of the containers in the KXMLGuiFactorygets deleted unnoticed.
Comment Actions
@kossebau Akademy is in full swing, and we try to get the review requests down. Could you move this forward? Would be really nice :-)
Comment Actions
I won't be available the following three weeks. If you feel like committing, please go ahead.
Comment Actions
In my opinion this should be committed, since it fixes a few crashes next to improving the file mode identification.
Comment Actions
I fear I have mentally moved too far away from this plugin for a qualified comment, sorry.