feat(MBS-14442): implement Guess Punctuation button with single quote conversion logic - #3833
feat(MBS-14442): implement Guess Punctuation button with single quote conversion logic#3833arsinclair wants to merge 2 commits into
Conversation
… conversion logic - my first PR here, so please let me know if there's something I missed; - this is narrowly scoped to single quotes detection and replacement only; once I'm confirmed this is the direction we want to move in, I'll extend the logic to double quotes, dashes, ellipses, etc in future PRs.
| // Match quote pairs only when they are bounded by non-word characters. | ||
| .replace(/(^|[^\p{L}\d])'(.+?)'(?=[^\p{L}\d]|$)/gu, '$1‘$2’') | ||
| // Any remaining single quotes are apostrophes. | ||
| .replace(/'/g, '’'); |
There was a problem hiding this comment.
I would actually advice against using this as the last step in the official MBS implementation. It also converts weird edge cases which should better be left alone.
This was OK for a userscript implementation, although I wanted to improve this for some time now, but never got to it.
There was a problem hiding this comment.
This is the only logic that handles single apostrophes, if we remove it we won't be able to handle them. If we change the order, it will affect paired apostrophes.
Of course, there's a always a way of going forward with the whitelist, e.g. 's[\s^], n't[\s^] etc, but I don't know if this is necessary at this point. I suppose it would be helpful to see some examples of those edge cases if you still have them?
|
Hey @mwiencek, this has been sitting here for more than a week. Does the review normally take that long, or is there any changes needed in the PR? |
|
We're a small team and MusicBrainz specifically is even more time constrained than usual right now, so things can take a bit, sorry :) |
|
No worries, I will work on my patience 😁 |
|
If it works, can you send a pull request to us for that too? It sounds useful 🫣 |
A pull request for what? This is the only pull request in question, I think. |
|
For patience I meant! 😂 |
|
LOL, it took me a while to get it hahahaha Once I have figured it out, I'll be sure to open a PR for the patience |
4 participants
Problem
MBS-14442.
Add a Guess Punctuation button similar to the existing Guess Case functionality.
When clicked, the button should automatically detect ASCII punctuation in entered titles and replace it with the appropriate preferred Unicode punctuation where applicable.
This issue is intentionally limited to implementing the punctuation-guessing/replacement functionality. Highlighting changes, character-level diffs, undo support, and improvements to Guess Case should be handled separately.
This is a smaller scoped part of MBS-11646.
Solution
The conversion logic, as well as the collection of test cases is based on (or entirely copied from) @kellnerd's Guess Unicode punctuation script, which is amazing and I've been using it for years.
Most of the logic is just a replica of Guess Case functionality with a different handler function just for punctuation.
One oddity to mention, in https://github.com/metabrainz/musicbrainz-server/pull/3833/changes#diff-f013e29e5dd3ed65921c393fa6ec6b329e2929a36fd87b67c2a4cc602f180557R449 the spacing between buttons is achieved with a literal white space character and its size is not constrained to integer pixels. So whereas 9px space allocation worked for 3 buttons, for 4 buttons setting it to 12px is not enough, since the width of a single white space is 5.28px.
So I had to set it to 13px.
Screenshots:
AI usage
Yes, plumbing the new logic based on the existing Guess Case functionality, adding tests and full review.
Testing
All automated tests were ran by the LLM.
Manual testing and polishing done with a local musicbrainz docker container.
Documenting
🔰 If you updated documentation pages, mention them here, such as:
Updated WikiDocs page
Not yet, I need a confirmation first if this is the right direction.