Closed quinox closed 1 month ago
In a post XZ world it's rather rude to dump so much code into a PR. I removed the inclusion of edlib
from my branch and implemented the Wagner–Fischer algorithm algorithm from scratch. When you read the code you won't easily understand why the code gives the right answer, but it is clear to see it's not doing anything scary or dangerous. The algorithm itself is well documented online, a visual demo can be found here (via this page)
In a post XZ world it's rather rude to dump so much code into a PR. I removed the inclusion of
edlib
from my branch and implemented the Wagner–Fischer algorithm algorithm from scratch. When you read the code you won't easily understand why the code gives the right answer, but it is clear to see it's not doing anything scary or dangerous. The algorithm itself is well documented online, a visual demo can be found here (via this page)
Great, that does address my hesitance. I like the feature, just not the big code dump.
I left some comments.
I made a branch with my review and enhancement:
https://github.com/halfgaar/FlashMQ/tree/quinox-feat-config-suggestions
If you're OK with it like this, I can squash and merge.
I definitely have stuff to learn with C++.
It's all good, squash and merge away.
Alright, it's in master :+1:
Kind of ready if you don't mind me hacking around in
FMQ_COMPARE
This branch modifies FlashMQ so that it will suggest possible fixes when an unknown configuration keyword is found. For example, when the user makes a typo and writes
session
instead ofsessions
:The way it works: when an unknown key is found a list of all valid keys within that context are compared to the unknown key, and the one with the shortest reasonable Levenshtein distance is suggested. This distance expresses how many insertions/substitutions/deletions are needed to go from 1 word to another. In the example above the distance is 1: if you insert an
s
in the right place the words match.Open questions:
The library I used to calculate the distance,edlib
, is licensed under the Expat license ("MIT"). I think it's fine for you to use it like this but I'm not a lawyerThis feature adds a lot of code, primarily in theedlib
. The lib looks clean and well documented but I'm not capable of assessing its quality / exploitability. If you want to replace it with another library, now or in the future, it's quite straightforward though: any library that gives you a distance value can be plugged inIf you want this feature and this lib: how do you want to store it in the repo? git submodules, the current setup, a single3rdparty
dir that contains the.h
and.cpp
, throw the files in the root dir?FMQ_COMPARE
was okay for comparing short words but the output was very hard to read when asserting entire sentences. I therefore hacked thefmq_assert
method to support 2 types of output, one with newlines and one without newlines. This is rather ugly, but I'm not sure how else to solve it. I could create anFMQ_COMPARE_VERBOSE
with an extra underlyingfmq_assert_verbose
but that sounds like it will result in duplicate code etc. Perhaps I could add a boolean argument forfmq_assert
that activates when used with a newFMQ_COMPARE_VERBOSE
, would that be nicer?Before merging I'll have to rebase it on master, it's currently rebased on a feature branch of mine (which I anticipate will get merged into master soonish)the other branch has landed and this branch has been rebased on your master