Caster icon indicating copy to clipboard operation
Caster copied to clipboard

Allow for individual key/spec clashes without knocking out entire rules?

Open kendonB opened this issue 6 years ago • 4 comments
trafficstars

Is your feature request related to a problem? Please describe. We want to be able to have individual spec conflicts between rules and handle them by preferring rules that were enabled most recently. The big thing this allows us to do is to move commands out of app grammars and into global or many-context grammars then implement exceptions in individual context rules. This will help to remove some duplication in the code.

Describe alternatives you've considered From @synkarius: Yes, it is possible. Have a look at DetailsCompatibilityChecker and its unit tests.

DetailsCompatibilityChecker computes incompatibilities (and delivers that information to the merge strategy in the next step), but doesn't KO anything. In contrast, the now-used SimpleCompatibilityChecker does the opposite: KOs incompatible rules like old Caster did, but doesn't provide any information about why.

I will not add this feature, but I foresaw this request. Here is what I suggest:

Add a setting in the settings which re-enables DetailsCompatibilityChecker. This will get you all the incompatibility information at the merge strategy step. Use the same setting to enable a new merge strategy (because as we saw, DetailsCompatibilityChecker doesn't work well with ClassicMergeStrategy). This new merge strategy can handle spec incompatibilities in some other way than a KO.

Looking at it again, I forgot: DetailsCompatibilityChecker only tells you which rules are incompatible, not which specs. But that would be easy to add. Just add the incompatible specs information to the CompatibilityResult class. The rest of the work is already done.

Or alternatively, leave CompatibilityResult as it is, and deal with the incompatible rules entirely in the new merge strategy.

Additional context original issue https://github.com/synkarius/Caster/issues/50


Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

kendonB avatar Oct 27 '19 02:10 kendonB

We also need to decide whether this is worth pursuing when we could just use ContextAction within global grammars to achieve this deduplication. The negative with using ContextActions is that I think it makes it a bit trickier to make edits though it may be easy enough.

kendonB avatar Oct 27 '19 03:10 kendonB

Allowing for key/spec clashes will probably make putting in user rules with such conflicts much simpler for the user as well.

kendonB avatar Oct 27 '19 03:10 kendonB

I think it's a good way to move forward. I think it might be wise to be careful how much we all rely upon ContextAction as well as it makes it harder to read grammars and make edits and this will be especially the case with moving towards an API style.

Getting as much information about the merge conflicts will be useful. Perhaps after the basics are implemented we can even let the user choose which overrides.

LexiconCode avatar Nov 01 '19 18:11 LexiconCode

I added:

        "what rule":
            R(Text("Firefox")),

to my firefox rule and:

        "what rule":
            R(Text("Navigation")),

to my NavigationNon rule. Both rules happily coexist. When both contexts are active, i.e. when in firefox, the rule that was first enabled takes precedent. I can reliably switch them around without anything breaking.

Should Caster be allowing these incompatibilities for MappingRules?

It works fine for me here and it's controllable so that's good. However, I would have expected the most recently enabled rule to take precedence. Even better, I would want the system to always favor a context rule over a global one and if comparing two context rules, favor the most recently enabled.

kendonB avatar Nov 11 '19 08:11 kendonB