Enforce SyxtaxMapping::builtin() to not have duplicate keys

[YeungOnion]

Presently, SyntaxMapping is implemented as a Vec, but we could have duplicate keys. I can insert a pair that has the same Glob (and even the same string) into the mapping variable used by builtins().

crate::get_syntax_mapping_to_paths emits a hashmap, so I think that means we'll always get a syntax to map to if a key is duplicated to two different MappingTargets but in principle it'll depend on when it was added to the HashMap so that the Vecs order matters.

I don't see a need to change from Vec, but maybe testing that the Vec doesn't have matching globs in the first piece of the tuple would be good.

However, the list of builtins isn't long to read, so maybe this isn't worthwhile, but it's also unsorted. If this is something useful, I can add that test.

[keith-hall]

I like the idea of adding a test for it, it would increase confidence during reviews without having to read through all the syntax mappings

[YeungOnion]

Re: closing PR #2646
I started the branch for this test from the wrong commit and had some changes from a different PR I made (adding RON syntax #2642). I'm new enough to git where I felt more comfortable making a new branch from the commit before and checking out only the files related to this duplicate keys test.

Is it cleaner if I pop out the commits and changes from this branch or is it fine if I open a new PR related to this issue?

[YeungOnion]

As impl Eq for Glob is not equivalent to "the two globs match on only the same set of strings?" so we won't catch those that aren't the same string pattern. Probably fine, right?

[keith-hall]

As impl Eq for Glob is not equivalent to "the two globs match on only the same set of strings?" so we won't catch those that aren't the same string pattern. Probably fine, right?

Yep, that's probably what we want so more specific globs would take precedence over less specific ones.

Re: closing PR #2646 I started the branch for this test from the wrong commit and had some changes from a different PR I made (adding RON syntax #2642). I'm new enough to git where I felt more comfortable making a new branch from the commit before and checking out only the files related to this duplicate keys test.

Is it cleaner if I pop out the commits and changes from this branch or is it fine if I open a new PR related to this issue?

No worries, everyone has to start somewhere and learn somehow 🙂 I'd be fine with a force push to the existing PR branch to drop commits, but GitHub might not let you reopen the PR in some cases doing this, so probably a new PR is less hassle for you in the long run