Skip to content

Conversation

@qomarhsn
Copy link
Contributor

This PR refreshes BnKhiproCombiner to align with the newest official Khipro release, keeping the mapping behavior consistent and up to date.

@Helium314
Copy link
Owner

Do you think it makes sense to put the mappings into some text file? This might allow for copy/paste when updating the mappings, depending on the source file.

@qomarhsn
Copy link
Contributor Author

That would make things easier for me as well. I’m open to restructuring, but I’m not sure what the process should be. Should the app load the mapping file during build, or should it be parsed manually in the code? And which format would you prefer—JSON or plain text?

A brief direction would help me implement it correctly

@Helium314
Copy link
Owner

My idea was to load the file on creating the combiner (maybe just once and then keep the result in the companion object).
Both Json and plain text would be fine. Ideally we would just use the source format, what does it look like?

@qomarhsn
Copy link
Contributor Author

The source mapping is an m17n .mim file (bn-khipro.mim). It’s plain text with (map …) and (state …) sections mapping Latin sequences to Bengali characters, for example ("k" "ক") or ("r" "র"). Full file here: https://github.com/rank-coder/khipro-m17n/blob/main/bn-khipro.mim

Since the .mim layout was designed for PC, a few mappings that don’t make sense on mobile (like the numeric ongko mappings such as 1 → ১, 2 → ২) were omitted. Otherwise the implementation follows the original source closely.

Because Khipro is state-based, every mapping belongs to a state, and not all states are needed on mobile. One possible approach would be to convert the .mim mappings into a small JSON structure that keeps the same state layout, for example:

{
  "SHOR": { "o": "", "a": "" },
  "FKAR": { "fa": "", "fi": "ি", "fii": "" }
}

The combiner could load this JSON once, build the internal tables, and handle state transitions exactly as defined by the source. This would make updates simple and keep the structure clear. If this direction fits the project, I can implement it in this PR.

@Helium314
Copy link
Owner

Since the .mim layout was designed for PC, a few mappings that don’t make sense on mobile (like the numeric ongko mappings such as 1 → ১, 2 → ২) were omitted. Otherwise the implementation follows the original source closely.

Ah, so just copying (part of) the file might not be suitable.

The combiner could load this JSON once, build the internal tables, and handle state transitions exactly as defined by the source. This would make updates simple and keep the structure clear. If this direction fits the project, I can implement it in this PR.

If this works for you and simplifies future updates I think it would be good.
But I don't want to force you to do that, in case you prefer the current style just tell me and I'll merge the current PR.

@qomarhsn qomarhsn changed the title Sync BnKhiproCombiner with latest official Khipro mapping updates Refactor Khipro combiner to load mappings from json instead of hardcoded values Dec 1, 2025
@qomarhsn
Copy link
Contributor Author

qomarhsn commented Dec 1, 2025

Refactor done. The combiner now loads all mappings from JSON. The JSON is generated from the Khipro source repo and will be kept updated there: https://github.com/KhiproTeam/Khipro-Mappings. The file output/touchscreen.json in that repo already includes the needed removals and applied adjustments.
For future Khipro mapping updates, I’ll just copy that file into app/src/main/assets/khipro-mappings.json and it will be ready.

One idea I wanted to ask about: is it possible to add that mapping repo as a git submodule so it stays synced automatically during the build?

…, improve consistency, and fix a minor Reph-state error.
…ayout for user-customizable choice; map Shift → '/' and add ';' after space to follow official Khipro touchscreen adaptation rules.
@qomarhsn qomarhsn changed the title Refactor Khipro combiner to load mappings from json instead of hardcoded values Refactor Khipro combiner to load mappings from json instead of hardcoded values and update functional layout Dec 5, 2025
@Helium314
Copy link
Owner

Thanks, this is a good solution!

One idea I wanted to ask about: is it possible to add that mapping repo as a git submodule so it stays synced automatically during the build?

What I'd prefer is adding it to release.py, which I run before each release to make sure I don't forget stuff like updating translations or adding changelogs.

@qomarhsn
Copy link
Contributor Author

qomarhsn commented Jan 7, 2026

OK then, that will be good. The mappings will be managed by the release.py script by copying output/touchscreen.json from Khipro-Mappings to app/src/main/assets/khipro-mappings.json in HeliBoard.
I’ll only need to open a PR when the combiner itself needs an update. That makes my work easier.

@Helium314
Copy link
Owner

Actually I hadn't noticed before that now there are separate functional keys for Khipro again. Why is that?

@qomarhsn
Copy link
Contributor Author

qomarhsn commented Jan 8, 2026

Khipro recently updated its default touchscreen layout, where / remains in the Shift position and ; is placed in the bottom functional row after the space key, and most users are comfortable with this default setup; however, some users shared feedback that they want to reverse ; and /, while others prefer not to have extra keys in the bottom row and instead want ; in the Shift position with / available as a popup, so separate functional layouts were added again to let users choose their preferred option, making key placement more flexible and allowing users to customize the layout according to their own typing habits.

@Helium314
Copy link
Owner

Ok, got it. I'm not happy about it because it somewhat interferes with existing custom layouts, but looks like there is no better solution.

A few things:

  • I'd prefer a separate PR for such unrelated changes (combiner mappings and functional keys layout) next time
  • please use the normal in-app logging instead of bypassing it by only using android.system.Log
  • don't use application context, instead use the IME service context via either Settings or KeyboardSwitcher. Neither of those fit 100% for some getKhiproMappings method, but it wouldn't be the first not-quite-suitable functionality in there.
  • why did you write KhiproMappingLoader instead of using e.g. Json.decodeFromString<MutableMap<String, Map<String, String>>>(jsonString)?

@qomarhsn
Copy link
Contributor Author

qomarhsn commented Jan 9, 2026

Understood.

Next time I’ll always try to make separate PRs for each unrelated change.

The other points are addressed in the latest commits: in-app logging is used, the application context was replaced with the IME service context via Settings, and the extra loader was replaced with direct JSON decoding.

Thanks for the review.

@Helium314 Helium314 merged commit 28beeea into Helium314:main Jan 10, 2026
@qomarhsn qomarhsn deleted the khipro-update branch January 10, 2026 10:37
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