fix: freeze built-in modes to prevent grammars altering them #2271
Conversation
- previously a ill-behaved grammar could change the global modes - this prevents that behavior
|
Technically this would be a breaking change for any BADLY behaved 3rd grammars that were taking advantage of this loophole before... That doesn't really both me a lot (since I'm assuming it's a small number or maybe zero). I'm also not convinced we need a MAJOR release for this. If you don't think we can accept this kind of potential breakage in a minor release then I suppose my preference would be to isolate the compile steps so that we just refuse to compile poor grammars and keep working with the good grammars. (although that's also a -slightly- breaking change, depending on your definition of breaking) IE, so then if someone used a "broken" grammar it would just highlight as "plaintext" (ie, no highlighting) with the next version, and they would have to fix their broken grammar. This was on my list of things to do eventually, but wasn't planning on getting to it until later. Thoughts? |
Well, let's keep the breaking changes (potential or not) for major releases.
Are we in a hurry? Why not just wait for some time with this PR until the next major release? |
Well, I tend to have had poor experiences with long-lived PRs in general. There are git strategies to make dealing with such things easier, but still I've always found it's best to get things finished up and committed, when possible. Also, technically right now any language grammar loaded can break ALL the other grammars by subverting the constant utility rules (either on purpose or on accident). Seems kind of bad. How do you feel about my suggestion if I just go ahead and add a catch so that any 3rd party languages behaving poorly get "disabled" (with a console error) but all other languages continue working just fine? Would that be a let us get this merged now vs later? IE, I'd just place a try/catch in |
I think it returns us back to #2276, because you don't want an error to get swallowed when developing new grammar. As I can see in #2273, there are quite a few breaking changes for v10. Maybe these changes worth a separate branch? |
Yes, I'm fine with making that a requirement here. But if we make that a thing, do you think this is safe to go in and the default "production" mode would then be that broken languages (which there might not even be any!) are just disabled, while other grammars continue to work just time.
After 9.17 I'll probably make a 9_STABLE branch or similar and then master will become the 10 branch. I was just writing down the list in an issue so I don't forget things. :) |
I think it's OK in this case. |
|
@egor-rogov Merging this now that #2286 has hit. |
Yep. |
No description provided.