Skip to content

fix: avoid label/search-key conflicts by checking continuation#1

Open
kanlac wants to merge 1 commit into
shitcoding:mainfrom
kanlac:pr/label-conflict-skip
Open

fix: avoid label/search-key conflicts by checking continuation#1
kanlac wants to merge 1 commit into
shitcoding:mainfrom
kanlac:pr/label-conflict-skip

Conversation

@kanlac

@kanlac kanlac commented Feb 20, 2026

Copy link
Copy Markdown

Summary

  • Replace next-character heuristic with continuation-based conflict detection.
  • Exclude only label keys that actually keep the current search valid.
  • Keep label assignment logic unchanged otherwise.

Why

In some cases (especially with case-sensitive searches and custom alphabets), next-character filtering can over/under-filter label keys. Checking real search continuation matches flash.nvim behavior more closely.

Test

  • npm run test:unit -- FlashMatchDetector.test.ts
  • Added regression: case-sensitive mode should not over-exclude labels.

@shitcoding

Copy link
Copy Markdown
Owner

So sorry I didn't address your PR for so long - just noticed it, for some reason I didn't receive notification about it.
And to be honest, I didn't think anyone would give a shit about this plugin, so I didn't check PRs/issues on github page:)

Will check your PR now and get back to you.

@shitcoding

Copy link
Copy Markdown
Owner

Okay, went through it properly — and I also poked around your fork (https://github.com/kanlac/obsidian_flash), so I've seen the Shuangpin input mode and the refined version of this fix. Lots to like here.

On the original fix: you're right that the old next-character heuristic over/under-filters in case-sensitive mode with custom alphabets, and checking real search continuation is the more correct approach (closer to flash.nvim too). The regression test nails exactly what was broken. I want this in — and I noticed 0ce8a5b on your fork is the cleaner, generalized version of it (routed through getRawMatches, works for both modes), so that's the one I'd build on rather than the original PR commit.

The catch is timing. The plugin sat untouched for a while, then I went through community-plugin review and pushed a batch of fixes — one of them (d910f7e, "Cyrillic keyboard bug") rewrote the exact block this touches, which is why GitHub now flags the PR as conflicting. That commit also added keyboard-layout–aware label exclusion, and it's worth being explicit about the intent here: the layout handling isn't meant to be Russian-specific.
The goal is non-Latin support generally — Cyrillic was just the first one I tackled because it's what I use — so the plugin isn't an en/ru-only thing. Which is exactly why your Shuangpin mode is so on-target.

The mechanism is deliberately layout-agnostic: label selection already reads physical key codes, and continuation-exclusion uses the Keyboard Layout Map API with a Cyrillic fallback.
Your continuation check tests searchString + <latin candidate>, which doesn't see that layout mapping, so on rebase we'd need to fold it back in (test the char the physical key actually produces, not just the Latin one), and I'll tighten the weak Cyrillic test so it can't silently regress.

On the zh-pyjj mode — I really like it, and it fits the direction perfectly. The opt-in setting, isolated pyjj.ts, IME guard, and the per-match matchLength refactor in the widget are all clean (that last one I'd want anyway). The one thing I need to work through before merging is bundle size: pinyin-pro is ~930 KB unpacked and our shipped main.js is currently ~400 KB, so I want to measure the real impact and figure out whether to ship it for everyone or load the dictionary more lazily.
Not a no — just a "let me get the size story right first."

So the plan I'd suggest: rebase onto current main, base the conflict-fix on your 0ce8a5b, fold the layout-aware exclusion back in, and bring the Shuangpin mode along once I've sorted the bundle question.
Happy to do that rebase/rework myself and credit you on the commits, or review if you'd rather drive - let me know what you decide.

Either way — genuinely didn't expect anyone to care about this little plugin, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants