Skip to content
New issue

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

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

Already on GitHub? # to your account

refactor(InputMacro): introduce InputMacro classes for grouping #99

Merged
merged 4 commits into from
Dec 18, 2023

Conversation

xatier
Copy link
Contributor

@xatier xatier commented Dec 18, 2023

This commit introduces a few InputMacro classes to avoid repetitive
code. Most of the replacement functions are doing the same with different
set of parameters, we can move them to a parent class.

  • Use vector.size() to avoid a few magic numbers.
  • Use std::unique_ptr for icu objects
  • Mark virtual method overrides properly.
  • Add clang-tidy checks to catch virtual function issues.

Add short keywords for the time macros as discussed in
#98 (comment)

refactor(InputMacro): provide formatWithPattern and formatWithStyle

  • extract common tasks from formatDate and formatTime into formatWithStyle
    for more generic formatting.
  • add a new formatWithPattern helper to format datetime with patterns
  • implement THIS_YEAR/LAST_YEAR/NEXT_YEAR macros with formatWithPattern

feat(InputMacro): implement weekday macros

This commit introduces a few InputMacro classes to avoid repetitive
code. Most of replacement functions are doing the same with different
set of parameters, we can move them to a parent class.

* Use vector.size() to avoid a few magic numbers.
* Use std::unique_ptr for icu objects
* Mark virtual method overrides properly.
* Add clang-tidy checks to catch virtual function issues.
@xatier
Copy link
Contributor Author

xatier commented Dec 18, 2023

@lukhnos this is the follow-up from #98 (comment), please kindly take a look.

It'd be nice if my proposal of short keywords for the time macros can be back ported to the upstream.

@xatier
Copy link
Contributor Author

xatier commented Dec 18, 2023

Just popped up another idea that we can add some more macros:

今年 -> 2023 / 2023 年 / 民國112年 / 令和5年
今天 -> 星期一 / 禮拜一

Would that be a good idea?

* extract common tasks from formatDate and formatTime into formatWithStyle
  for more generic formatting.
* add a new formatWithPattern helper to format datetime with patterns
* implement THIS_YEAR/LAST_YEAR/NEXT_YEAR macros with formatWithPattern
@xatier
Copy link
Contributor Author

xatier commented Dec 18, 2023

While implementing the year macros, I noticed that we can apply some further extractions to make our helpers more generic.

@xatier
Copy link
Contributor Author

xatier commented Dec 18, 2023

screenshots from my local build

image
image
image

@xatier
Copy link
Contributor Author

xatier commented Dec 18, 2023

With the pattern stuff, implementing weekday macros is trivial.

image
image

Copy link
Collaborator

@lukhnos lukhnos left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

I filed #100 to discuss the maintenance challenges that arise from this feature, which needs to be maintained on both source code and data levels in both McBopomofo macOS and fcitx5-mcbopomofo. For now let's take the diffs.

@lukhnos lukhnos merged commit ad11e21 into openvanilla:master Dec 18, 2023
3 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants