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.

Sign up for GitHub

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

Jump to bottom

Add/improve various symbols #933

Merged
merged 9 commits into from
May 29, 2024
Merged

Add/improve various symbols #933

merged 9 commits into from
May 29, 2024

Conversation

tats-u
Copy link
Contributor

@tats-u tats-u commented May 19, 2024 •
edited
Loading

Description

A clear and concise description of this pull request.

Added/improved symbols composition:

  • 合成用濁点・半濁点
  • ダッシュ（em・en・水平線）
  • 縦線（双柱・平行の区別）
  • カッコ・引用符ペア
  • 数学記号（±・∓・≠・≦・≧・三点リーダー）
  • 将棋駒
  • 麻雀牌
  • 発音記号（英語用）
  • 矢印（改行・斜め）
  • 上付き・下付き文字

Issue IDs

Issue and/or discussion IDs related to this pull request.

#929

Steps to test new behaviors (if any)

A clear and concise description about how to verify new behaviors (if any).

  • OS: Any
  • Steps:
    1. Input
  1. Convert
  2. Confirm you can get the correct symbol

Additional context

Add any other context about the pull request here.

I split commits according to symbol types.

hiroyuki-komatsu
hiroyuki-komatsu requested changes May 20, 2024
View reviewed changes
Copy link
Collaborator

@hiroyuki-komatsu hiroyuki-komatsu left a comment

Choose a reason for hiding this comment

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

Thank you for the PR!
I added comments to the changes.

src/data/symbol/symbol.tsv Outdated Show resolved Hide resolved
src/data/symbol/symbol.tsv Outdated Show resolved Hide resolved
src/data/symbol/symbol.tsv Show resolved Hide resolved
src/data/symbol/symbol.tsv Outdated Show resolved Hide resolved
src/data/symbol/symbol.tsv Outdated Show resolved Hide resolved
src/data/symbol/symbol.tsv Outdated Show resolved Hide resolved
@tats-u
Copy link
Contributor Author

tats-u commented May 20, 2024

I wonder when we should edit categorized.tsv or ordering_rules.tsv.
I don't feel like editing them in the honest if possible.

hiroyuki-komatsu
hiroyuki-komatsu reviewed May 21, 2024
View reviewed changes
Copy link
Collaborator

@hiroyuki-komatsu hiroyuki-komatsu left a comment

Choose a reason for hiding this comment

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

Thank you for the updates.

For categorized.tsv or ordering_rules.tsv, you do not necessarily need to update them.

src/data/symbol/symbol.tsv Outdated
記号 ≤ ふとうごう しょうなりいこーる すうがく < <= きごう 不等号 海外 SYMBOL
記号 ≥ ふとうごう だいなりいこーる すうがく > >= きごう 不等号 海外 SYMBOL
記号 ⩽ ふとうごう しょうなりいこーる すうがく < <= きごう 不等号 海外 SYMBOL
記号 ⩾ ふとうごう だいなりいこーる すうがく > >= きごう 不等号 海外 SYMBOL
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess these 海外 symbols are not so popular, but confusing with ≦ and ≧.
How about removing them?

Copy link
Contributor Author

@tats-u tats-u May 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We can frequently find them in higher education even in Japan, I think.
"大学" or "大学・海外" instead of "海外" might be better.
Do you still think they're unnecessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

@yukawa yukawa May 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Do you still think they're unnecessary?

I don't say they are unnecessary, but given that those we often use these characters can easily add them into user dictionary, I'd say the urgency is not that high. Actually, I'm more interested in how we can improve our candidate window UX as we keep increasing number of candidate, but it'd be a different story (*).

(*): Try to convert えもじ or きごう. I feel the number of candidates is already way too big. We even hit #741 because of that.

Copy link
Contributor Author

@tats-u tats-u May 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

(*)

I guessed it. I'll remove "きごう" from the following symbols because they'll not be wanted by those who try to input symbols by "きごう".

  • the above 4 inequality symbols
  • mathematical dots

I don't know whether we should do the same for:

  • ∓
  • en dash
  • (em dash) (would need almost always to be used instead of U+2015 if Windows didn't exist)

Copy link
Collaborator

Choose a reason for hiding this comment

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

the above 4 inequality symbols

For those 4 inequality symbols, I'd suggest not including into this PR at least until we come up with a better description. For people who have never used those characters, I guess "大学", "大学・海外" and "海外" are all hard to understand what the description tried to explain. Let's revisit them later, e.g. 1 year later for example. Again, I don't think adding them are not that urgent.

Copy link
Contributor Author

@tats-u tats-u May 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Removed it as you wish.
Also removed "きごう" from mathematical dots because they're not in JIS X 0213.
I found both dashes and ∓ are all in JIS X 0213, so I thought I don't have to remove "きごう" from them.

src/data/symbol/symbol.tsv Outdated Show resolved Hide resolved
src/data/symbol/symbol.tsv Outdated Show resolved Hide resolved
@tats-u
Copy link
Contributor Author

tats-u commented May 22, 2024

For categorized.tsv or ordering_rules.tsv, you do not necessarily need to update them.

I got it. I'll revert their changes.

@hiroyuki-komatsu hiroyuki-komatsu merged commit 483c9e1 into google:master May 29, 2024
1 check passed
@hiroyuki-komatsu
Copy link
Collaborator

Thank you for updating the change! It looks good to me too.
We have merged you PR.

After some actual experiences, we might adjust some entries or orders in future.
Best regards,

@tats-u tats-u deleted the symbols branch May 29, 2024 09:43
hiroyuki-komatsu added a commit that referenced this pull request Jun 6, 2024
@hiroyuki-komatsu
Update BUILD_OSS to 5490
26e3a83 
Features
* Added more symbol characters (#933)
* Added words of 対象内 / 対象外 by fixing the format error (#928)

Bug fix
* Fixed the crash issue on saving the user dictionary. (42cbb3f)
* Fixed the crash issue on the debug build caused by converting negative numbers (#878)
* macOS: Replaced "Google Japanese Input" with Mozc in UI messages. (caeb9ce)
* macOS: Fixed the crash issue of mozc_emacs_helper (#900)
* macOS (Apple Silicon): Fixed the crash issue of GUI tools (#791)

Build
* Set proper release/debug build options for GitHub Actions (07c5f89)
* Updated the Protobuf version: v26.1 → v27.0 (#937)
* Updated the Qt version: 6.7.0 → 6.7.1 (#934)
* Linux: Added Dockerfile for Ubuntu 24.04 (#924)
* Windows: Simplified the build process by reducing manual execution of the vcvars .bat file. (#923)

Code
* Applied Clang's build cleaner
* Completed the migration from base/logging.h to Abseil log.
* macOS: Applied suggestions from code scanning alerts (b955086)

PiperOrigin-RevId: 639975159
@hiroyuki-komatsu hiroyuki-komatsu mentioned this pull request Jul 2, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yukawa yukawa yukawa left review comments

@hiroyuki-komatsu hiroyuki-komatsu hiroyuki-komatsu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tats-u @hiroyuki-komatsu @yukawa