fix: clicks no longer silently deleted when adjacent to a modifier

[varunghat]

Fixes clicks getting silently deleted when reordering an adjacent modifier
(e.g. kǀʰʷ -> kʷʰʷ) in order_ipa()

Clicks were typed the same as modifiers ("M") in make_typestring()
Gave clicks their own typestring code ("K").

Doesn't touch feature-helper-functions.R's separate click="M" typing, so feature computation is unaffected.

[drammock]

I checked git blame for this line, which led me to #365. It's PR description says (among other things):

when building feature vectors, clicks are now handled like modifiers (not like base glyphs), which makes it easier to get the correct features (previously they were incorrect)

SO I think this change will likely result in incorrect feature vectors for clicks. I'd rather try changing the line in order_ipa that checks for modifiers %in% mods (see other comment)

[varunghat]

I checked the diff between output files generated with the original code, previous fix (clicks as "K") and this suggested fix. There's a few things to note:

1. The feature vectors do change, but worth noting the original (unfixed) code does not produce a valid phoible.csv file that matches the one in the repo. Running a fresh install crashes in add-features.R with the following error:
   GlyphID 0353 (frictionalized) used on base glyph k was not handled.

This probably means the current phoible.csv was obtained by running add-features.R on a pre-existing phoible-nofeats.csv?

2. The original code and the suggested fix change the ordering of some modifiers. For example: in kǃ̠ʰ (Present in HAZDA and SANDAWE), the original code changes it to k̠ʰʰ, then crashes with the above error. The suggested fix moves the diacritic onto the base k instead (k̠ǃʰ). This implies the feature vectors will be different but I'm not sure if this is intended behaviour.

This is because the above fix only solves for "MM" (two modifiers appearing sequentially) and not for "MD" (which is the above example - "BMDM")
So we need to further add in line 302 under while (stri_detect_fixed(typestring, "MD"))

            if (string[ix] %in% clicks) break

(Please check the comment below)

3. Finally, I compared the files generated with the previous fix (clicks as "K"), this complete fix and the phoible.csv currently committed in the repo: All three are identical.

[drammock]

I think this is the correct fix

Suggested change

	string[span] <- c(clicks, modifiers)[c(clicks, modifiers) %in% mods]

[drammock]

@varunghat could you push whatever changes are needed get the code into the state that you think is easiest to understand/maintain that still preserves parity with the current state of the output file (phoible.csv)? Then ping me and I'll look again / merge.

[varunghat]

@drammock Latest commit on this PR has the change which is probably easiest to understand/maintain, with minimal code change. Added some comments to explain this edge case. Still preserves parity with the current state of the output file phoible.csv .

[varunghat]

@drammock The "suggested" code change was more of a comment (while loop -> if), not meant to be a syntactically correct drop-in replacement. My bad for posting it as a suggestion block instead of a plain code block.

Current code is now syntactically incorrect and throws an error. Created a hotfix #390 with correctly formatted code, verified with regenerating phoible.csv, still preserves parity with currently committed phoible.csv