Don't crash on malformed syntax highlighting files
ClosedPublic

Authored by davispuh on Mar 5 2019, 12:07 AM.

Details

Test Plan
  1. Create malformed syntax highlighting file with missing end tags
  2. Place it in ~/.local/share/katepart5/syntax/
  3. Open Kate with file which uses that syntax highlighting

Diff Detail

Repository
R39 KTextEditor
Branch
malformed
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 9211
Build 9229: arc lint + arc unit
davispuh created this revision.Mar 5 2019, 12:07 AM
Restricted Application added projects: Kate, Frameworks. · View Herald TranscriptMar 5 2019, 12:07 AM
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel. · View Herald Transcript
davispuh requested review of this revision.Mar 5 2019, 12:07 AM
davispuh updated this revision to Diff 53235.Mar 5 2019, 8:24 PM

Allow invalid Format()

davispuh updated this revision to Diff 53236.Mar 5 2019, 8:26 PM

Whoops...

In principle ok, but I would like to have the m_properties.resi.... code in that if shared.
We already have that, just a few lines above.
You just need to move the computation of the auto definitions = definition().includedDefinitions(); a few lines up, you can just call that on the passed def.

cullmann requested changes to this revision.Mar 5 2019, 8:28 PM
This revision now requires changes to proceed.Mar 5 2019, 8:28 PM

And btw., thanks for taking care of that crash, it's ugly to crash for such things. My fault, did never test that :/

davispuh updated this revision to Diff 53237.Mar 5 2019, 8:38 PM

Don't duplicate code

Hmm, I just thought that might be correct, but doesn't that kill highlighting for all definitions that include none?
If forgot that includedDefinitions() doesn't include itself, that check makes no sense ;=)

I think we are missing some check at the end if we had no formats at all.

cullmann requested changes to this revision.Mar 5 2019, 8:44 PM

Sorry that I was confused, but I just have seen your check of definitions.isEmpty() and thought that would be the right one.

This revision now requires changes to proceed.Mar 5 2019, 8:44 PM
davispuh updated this revision to Diff 53970.Mar 15 2019, 5:43 PM

Also check for present formats

cullmann accepted this revision.Mar 16 2019, 3:31 PM

That looks reasonable, if the definition doesn't have any included ones + no own formats it is likely to be broken ;=)

This revision is now accepted and ready to land.Mar 16 2019, 3:31 PM
This revision was automatically updated to reflect the committed changes.