D23034: implicit fallthough if there is fallthoughContext

cullmann closed this revision. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D23034 To: jpoelen, #framework_syntax_highlighting, dhaumann, cullmann Cc: nibags, kwrite-devel, kde-frameworks-devel, LeGast00n, domson, michaelh, ngraham, bruns, demsking, cullma

D23034: implicit fallthough if there is fallthoughContext

cullmann accepted this revision. This revision is now accepted and ready to land. REPOSITORY R216 Syntax Highlighting BRANCH auto_fallthrough (branched from master) REVISION DETAIL https://phabricator.kde.org/D23034 To: jpoelen, #framework_syntax_highlighting, dhaumann, cullmann Cc: nibag

D23034: implicit fallthough if there is fallthoughContext

cullmann added a comment. Some whitespace stuff, git diff -w looks OK. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D23034 To: jpoelen, #framework_syntax_highlighting, dhaumann, cullmann Cc: nibags, kwrite-devel, kde-frameworks-devel, LeGast00n, domson,

D23034: implicit fallthough if there is fallthoughContext

dhaumann accepted this revision. dhaumann added a comment. I think this is much better. For KF6, we can remove it in the xml files as well or so... Besides that: why does the diff show so many changes even though it's just about fall through? It makes the review harder (especially on a s

D23034: implicit fallthough if there is fallthoughContext

jpoelen updated this revision to Diff 63455. jpoelen added a comment. - don't modify the *.xml files to maintain the compatibility in older versions of KF5 - langauge.xsd: accept only 1 or true for fallthrough attribute REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE http

D23034: implicit fallthough if there is fallthoughContext

jpoelen added a comment. I will hand over the original xml files, that seems preferable to me too. I did not see `fallthrough="false"` nor `fallthrough="0"`. But after checking, `brightscript.xml` uses` fallthroughContext` twice without `fallthrough`. in my opinion, it's an oblivion with

D23034: implicit fallthough if there is fallthoughContext

cullmann added a comment. Sounds like a reasonable solution. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D23034 To: jpoelen, #framework_syntax_highlighting, dhaumann, cullmann Cc: nibags, kwrite-devel, kde-frameworks-devel, univerz, LeGast00n, domson,

D23034: implicit fallthough if there is fallthoughContext

nibags added a comment. I think it's an improvement, but the ideal would be to maintain support for `fallthrough="bool"`. That is, if the `fallthroughContext` attribute is present, assume `fallthrough="true"`, unless "fallthrough=" is explicitly written. What I don't agree with is modif

D23034: implicit fallthough if there is fallthoughContext

cullmann added a comment. Hmm, isn't any file with some fallthrough=false but a fallthrough context just buggy? Did any of our files have that? REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D23034 To: jpoelen, #framework_syntax_highlighting, dhaumann, c

D23034: implicit fallthough if there is fallthoughContext

dhaumann requested changes to this revision. dhaumann added a comment. I like this change a lot, but there is a huge (theoretical) backwards compatibility issue with this. Before, the code ignored `fallthroughContext` if `m_fallthrough` was explicitly set to `false`. So you could have: f

D23034: implicit fallthough if there is fallthoughContext

cullmann requested changes to this revision. cullmann added a comment. This revision now requires changes to proceed. I appreciate the removal of the fallthrough=bool attribute. Given it is sematic-free if no fall through context is there I guess this is a "compatible" change. But please u

D23034: implicit fallthough if there is fallthoughContext

jpoelen created this revision. jpoelen added reviewers: Framework: Syntax Highlighting, dhaumann, cullmann. jpoelen added projects: Kate, Frameworks. jpoelen requested review of this revision. REVISION SUMMARY fallthrough="true" is redundant with fallthroughContext="xxx" and there is no reason