facelessuser / ApplySyntax

Syntax detector for Sublime Text
https://facelessuser.github.io/ApplySyntax/
343 stars 48 forks source link

Give priority to user-created rules over defaults #127

Closed michaelblyons closed 5 years ago

michaelblyons commented 5 years ago

This way, the main repository doesn't have to change every time someone discovers an exception to the default rules. For example, #82 and #125.

facelessuser commented 5 years ago

I am wondering if this will cause issues with people's expectations in some scenarios. I wonder if it would be better to somehow specify if you'd like your rule to come before or after...

michaelblyons commented 5 years ago

A setting to toggle which list comes first? That seems reasonable. If that's what you meant, I can cook one up.

facelessuser commented 5 years ago

I'm actually thinking of a way to specify a specific rule before or after. So you could do both with different rules. If that makes sense.

michaelblyons commented 5 years ago

It does, but several ways to do this all have their own weird idiosyncrasies:

  1. Have two lists that users can add to, e.g. prepend_syntaxes and syntaxes. Now the user (may) have another list to maintain, but this is clearly the easiest code to write.
  2. Allow a user to flag rules as "prepend": true. Now a user's syntaxes list depends on them to keep it in a sane order with all the prepended rules at the top. (Otherwise, it's like sorting a hand of cards from 2 to Ace and then deciding afterwards that clubs is trump.)
  3. Add support for a special placeholder "rule" in syntaxes that represents where the default_syntaxes are. If it does not exist, put default_syntaxes first to mimic existing behaviour. Communicating this in documentation might be a little weird.

Do you have a favorite, or some other plan that is better?

facelessuser commented 5 years ago

I think simple and straight forward is the way to go for this. I think adding a prefix list in addition to the postfix rule is more than sufficient. This gives you the option to add certain rules to capture before defaults or to only capture if it falls through the defaults. It preserved backwards compatibility while adding the new feature and allows flexibility just in case all prefix or all postfix isn't good enough. And it's pretty simple to add and support :slightly_smiling_face: .

facelessuser commented 5 years ago

Thanks for the update. I'm going to look closer at this because this line gives me pause: https://github.com/facelessuser/ApplySyntax/blob/138257dede7a1af7adf44a3b90a03cff52ef9411/ApplySyntax.py#L480. I'm wondering if their is a disconnect as to how we are processing things...In this case, we load project rules, then user rules, then defaults. But the place you are changing we always loaded default, then user...this looks confusing, and I'm wondering if I need to dig into this and clean some things up.

michaelblyons commented 5 years ago

I, too, should look more closely at that line. For one thing, I did not know that there were "project syntaxes" or how those look.

Edit: I'm not sure the line I tinkered with actually does what I want. In fact, I think the thing I wanted may have been here all along in the form of project_syntaxes as of https://github.com/facelessuser/ApplySyntax/commit/66666c36dac3e474a629cb5be237b26c11016008.

It's not working quite like I expected, though. I have now put this block both in ApplySyntax.sublime-settings and Preferences.sublime-settings and it's not working either place. I haven't put it into a .sublime-project, but I don't think that would be any different.

    "project_syntaxes": [
        {
            // If you have PackageDev installed, use it for tmPreference files
            "syntax": [
                "PackageDev/TextMate Preferences",
                "PackageDev/TextMate Preferences (PList / XML)",
                "PackageDev/TextMate Preferences/TextMate Preferences",
                "TextMate Preferences",
                "TextMate Preferences (PList / XML)",
                "TextMate Preferences/TextMate Preferences",
                "XML/XML",
            ],
            "extensions": ["tmPreferences"],
        },
    ]

I have also tried "syntax": "INI/INI" just in case none of the syntax names in my big stack was the right format.

Please do not merge anything until we get this straight. 😉

facelessuser commented 5 years ago

I know that project syntaxes don't use the extensions as that is a method that registers the extension via Sublime's extension association. We don't allow that in non global rules. Though, I don't see why we couldn't implement it in a different way....

I need to look at this more closely. I think you were updating the extension registration code. I'll try and take a look this week. It seems like we need some cleanup in this area.

michaelblyons commented 5 years ago

I know that project syntaxes don't use the extensions as that is a method that registers the extension via Sublime's extension association. We don't allow that in non global rules.

Yeah, I see that now in https://facelessuser.github.io/ApplySyntax/usage/#project-specific-rules. That rule did indeed start activating when I changed the extensions key to an equivalent file_path rule.

Amusingly, none of the (staggeringly large number of) possible package path/name combinations I entered appears to be the right one for that PackageDev syntax. The thing that does work is to set "syntax": [] in that rule, letting Sublime Text's normal behaviour take over.

I think you were updating the extension registration code.

Yes, I think I was.

facelessuser commented 5 years ago

Well, taking a closer look, it would appear that the user extensions should definitely match the user rules and come first. I'm going to look into why your patterns aren't making a difference though. I have a little time now, hopefully I can make sense of it.

facelessuser commented 5 years ago

Also, I think you can use extensions in project rules, they just won't be applied naively via Sublime's extension association. It should still apply the syntax though. Let me know if it doesn't though.

facelessuser commented 5 years ago

@michaelblyons, try out pull request #128 and let me know if that resolves all your issues.