[KTextEditor] Reset filetype when opening url
ClosedPublic

Authored by jsalatas on Feb 21 2017, 3:21 AM.

Details

Summary

Seems reasonable to me that when calling openUrl it should be reset to its original filetype and highlighting mode.

Test Plan

none

Diff Detail

Repository
R39 KTextEditor
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
jsalatas created this revision.Feb 21 2017, 3:21 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptFeb 21 2017, 3:21 AM
Restricted Application added a subscriber: kwrite-devel. · View Herald Transcript
jsalatas retitled this revision from Reset filetype when opening url to [KTextEditor] Reset filetype when opening url.Feb 21 2017, 3:22 AM
dhaumann accepted this revision.Feb 23 2017, 7:56 PM
dhaumann added subscribers: cullmann, dhaumann.

Looks reasonable to me as well... I don't see any negative side effects, so should be fine from my side.
@cullmann Can you confirm?

This revision is now accepted and ready to land.Feb 23 2017, 7:56 PM

Looks reasonable to me as well... I don't see any negative side effects, so should be fine from my side.
@cullmann Can you confirm?

Let's wait for a second opinion before committing this :)

Don't we need to honor "m_reloading" to avoid to do this on document reloads?

jsalatas added a comment.EditedFeb 24 2017, 7:06 AM

Don't we need to honor "m_reloading" to avoid to do this on document reloads?

KTextEditor::DocumentPrivate::documentReload() is keeping m_fileTypeSetByUser to a local variable (byUser) before calling openUrl and uses that local variable after reloading to set the highlighting mode.

I'll double check however and get back to you

jsalatas updated this revision to Diff 11800.Feb 25 2017, 3:22 AM

Honor "m_reloading" to avoid to avoid reseting the file type on document reloads as suggested by @cullmann

Don't we need to honor "m_reloading" to avoid to do this on document reloads?

You are right! We need to check m_reloading in cases of multiple consecutive reloads :)

jsalatas requested review of this revision.Feb 25 2017, 3:24 AM
jsalatas edited edge metadata.
cullmann accepted this revision.Feb 25 2017, 1:56 PM

;=) Feel free to commit that, thanks!

This revision is now accepted and ready to land.Feb 25 2017, 1:56 PM
This revision was automatically updated to reflect the committed changes.