Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update hebrew.json #738

Closed
wants to merge 1 commit into from
Closed

Conversation

russianspy1234
Copy link

Adding Nikud (Hebrew vowels) to resolve issue #735. Using the code from the Florisboard json file for Hebrew

Adding Nikud (Hebrew vowels)
@russianspy1234
Copy link
Author

This is my first ever pull request so apologies if I messed something up. I grabbed the json file from Florisboard found here:
https://github.com/florisboard/florisboard/blob/master/app/src/main/assets/ime/keyboard/org.florisboard.layouts/layouts/characters/hebrew.json

The nikud work just like other dialectics in unicode so it should just work.

@russianspy1234
Copy link
Author

Core functionality works. Not sure what the variation_selector part of the original json did, there was no equivalent that I could see in the version in florisboard, but everythign seems to work fine, even the . changing to an @ in an email address field.

Copy link
Contributor

@codokie codokie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/codokie/HeliBoard/blob/b7d55492553dfd21fb010f5ab6756191e8c2acb8/app/src/main/assets/layouts/hebrew.json

I made some changes to the layout:

  1. I added a few missing vowels
  2. now the vowels should appear by themselves except for 'ש' and 'ו' (vav) which commonly have a vowel applied to them

That is more consistent with GBoard approach for hebrew diacritic vowels.
Could you try it out? if you liked the changes then please update the json file. Thanks

@tenextractor
Copy link
Contributor

Isn't it better to update language_key_texts instead of the layout itself? This way it will apply to any Hebrew layout.

@russianspy1234
Copy link
Author

Isn't it better to update language_key_texts instead of the layout itself? This way it will apply to any Hebrew layout.

Maybe but if you look at the issue thread you'll see that there is disagreement about where the popups should go.

@codokie
Copy link
Contributor

codokie commented Apr 24, 2024

@tenextractor I don't think it matters, but it is less convenient to insert diacritics into a text file compared to a json which supports unicode format. Should you want to change the layout, you could use the current layout as a base and make changes to it.
@russianspy1234 Please review the suggested changes, Florisboard layout hasn't got it completely right.

@Helium314
Copy link
Owner

As there is ongoing discussion, I'm marking this as a draft.
Also, please stick to the HeliBoard style of Json layouts, and avoid adding codes when not necessary. This should also make the functional changes to the layout better visible.

Isn't it better to update language_key_texts instead of the layout itself? This way it will apply to any Hebrew layout.

I didn't properly check the layout, only looked at the diff. If e.g. אַ is added as popup to א, I agree.
But my understanding is that the PR only adds the characters that will be combined with the letter when both are entered sequentially.

@Helium314 Helium314 marked this pull request as draft April 25, 2024 19:31
@codokie
Copy link
Contributor

codokie commented Apr 25, 2024

i removed the codes from my layout:

If e.g. אַ is added as popup to א

in my layout this is the case only for 2 common letters, others just have the diacritic by itself

@Helium314
Copy link
Owner

So it seems the discussion stopped anyway after I had marked it as draft...

Just to summarize from here and issue #735:
A way of entering hebrew diacritics is wanted.
One proposal is adding them to one key in locale_key_texts/iw.txt, but this was not wanted / favored (?).
So the alternative would be taking the FlorisBoard layout either directly, or modified to show the "naked" diacritics (with exceptions for ש).

I'm certainly ok with adding the missing diacritics as popups, and I see the reasoning why the use of a showcase letter as done in the FlorisBoard layout is not wanted. For that reason I clearly tend to favor @codokie's layout (and because it sticks with no-unnecessary-codes for HeliBoard).
However, you both @codokie and @russianspy1234 seem to agree that the variation selector should be removed, even though I would assume those keys are completely unrelated to diacritics. Why is that?

@Helium314 Helium314 marked this pull request as ready for review May 19, 2024 14:33
@codokie
Copy link
Contributor

codokie commented May 20, 2024

I didn't explicitly state that the variation selector should be removed
It is now present in my layout although I personally don't find that really useful

@Helium314
Copy link
Owner

although I personally don't find that really useful

Mainly I don't want to change things people might be used without a good reason.
I also see usefulness as limited, but since URLs may contain Hebrew characters it still makes sense to keep the variation selector.

So @russianspy1234 I would like to use @codokie's layout, or do you have some objection on this?

@tenextractor
Copy link
Contributor

I think it's best to just copy the Gboard layout for this.

@russianspy1234
Copy link
Author

I have no strong feelings about which layout is used. I just saw a feature request I actually had the ability to complete and posted something. I rarely type in Hebrew and when I do it's usually without vowels. If I was building a layout from scratch I don't think I'd have done it like either of these: I'd put Bet as a popup of Vet / Pei as a popup of Fei, etc. I'd also probably want to put all of the sofits as popups on the regular letters, then all of the common vowels as a popup on one key and the uncommon vowels on another, but that's all getting into reinventing the wheel territory.

@Helium314
Copy link
Owner

Helium314 commented May 30, 2024

I would like to keep changes to existing layouts small. That reinventing-the-wheel thing could be done in a separate layout. But building a properly designed layout from scratch is probably not simple (assuming you want other people to use it because it's better).

@russianspy1234
Copy link
Author

I would like to keep changes to existing layouts small. That reinventing-the-wheel thing could be done in a separate layout. But building a properly designing a layout from scratch is probably not simple (assuming you want other people to use it because it's better).

Yeah that's why I figured it's better to use an existing one. I'm not gonna try to pull a Dvorak on a language I rarely type in

Helium314 added a commit that referenced this pull request May 30, 2024
see discussion in #738

Co-authored-by: codokie <@>
@Helium314
Copy link
Owner

Alright, I updated the layout now

@Helium314 Helium314 closed this May 30, 2024
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.

4 participants