SpatialMatch: Add AZERTY keyboard support

[brenard]

Hello,

I'm currently working on French support and this first PR add AZERTY keyboard layout support in SpatialMatch. My modifications:

• data-scripts/build_keyboard_adjacency_graphs.py:
  • switch to unicode to allow to handle not-ascii characters in keyboad layouts
  • make some assertion errors more explicit
  • add AZERTY keyboard layout
• src/Matchers/SpatialMatch.php: add AZERTY keyboard layout
• src/Matchers/adjacency_graphs.json: update it using new build_keyboard_adjacency_graphs.py script to add AZERTY keyboard layout

[mkopinsky]

Our approach in general has been to maintain feature parity (and more importantly, scoring parity) with the upstream dropbox/zxcvbn library. dropbox/zxcvbn#93 is the feature request there about AZERTY support. See #51 for previous discussion.

I think I might be OK with adding this in a feature flagged way, but as is it creates a score divergence between this library and the upstream JS library, which then creates UX problems when the frontend and backend of a website return different scores.

My preferred approach here would be to say that instead of tracking dropbox/zxcvbn as an upstream, we instead track some dominant fork that we can point people to in the documentation. I don't know if such a thing exists.

[alanpoulain]

I totally understand your point of view and I think it's important to have a feature and scoring parity between dropbox/zxcvbn and your project.

However there is a great fork of dropbox/zxcvbn: https://github.com/zxcvbn-ts/zxcvbn.
The scoring is different (see https://zxcvbn-ts.github.io/zxcvbn/guide/comparison/) because of some changes:

• more keyboard layouts
• behavior changes of matchers (for instance Add levenshtein distance zxcvbn-ts/zxcvbn#96)

Since the dropbox version is not maintained anymore, I think it would be great to have a scoring parity with this great fork.
It can be either a new library (this one can be maintained for bug fixes), a feature flag on this library or you can even make the choice to drop the parity from dropbox/zxcvbn.

What do you think?

[mkopinsky]

In principle, I think switching to feature/scoring parity with zxcvbn-ts makes perfect sense. I would do it as a new version rather than trying to feature flag it.

There's a few wrinkles we'd need to answer:

• Obviously this is a breaking change. Easily solved with a semver bump to 2.0. Which conveniently is also zxcvbn-ts's version (they're on 2.0.1). Long term, we have to decide how our version numbers relate to theirs - do we let them diverge? Keep the same major/minor, and increment the patch independently of them?
• zxcvbn-ts has multiple sub-packages (@zxcvbn-ts/core, @zxcvbn-ts/language-common, @zxcvbn-ts/language-en etc). I imagine the php library will still be a single package?
• So, who's going to do the work to bring it up to parity? :-) I don't have time right now to do this, but will review others' PRs.

[alanpoulain]

Thanks for your fast answer!

do we let them diverge?

Once you have bumped the version to 2.0, I think you can: some BC breaks are not the same than in JS (for instance if you bump the PHP version).

I imagine the php library will still be a single package?

Yes I think having multiple packages is more common in JS than in PHP (and there are more tools to manage multirepo like lerna or nx).

who's going to do the work to bring it up to parity?

I think the first step would be to create a next branch. We can then try to make some PR on it 🙂

[mkopinsky]

I just pushed up a next branch. Feel free to issue PRs against it.

[brenard]

I rebased my commits on your next branch and switch this PR to the next branch.