add language select, abstract text transformations #584
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…2' into text-transformation-tests
|
✔️ No visual differences introduced by this PR. View Playwright Report (note: open the "playwright-report" artifact) |
|
What is the delta you're seeing? The comment above is only showing N/A, not sure where to see accurate numbers. |
Casheeew
reviewed
Feb 12, 2024
themoeway-bot
previously approved these changes
Feb 12, 2024
Yeah, the changes to the test make it lose track, but it's gone from about 9 ms per lookup to about 24. |
Merged
|
RIP having to deconflict for the umpteenth time - #636 also added a setting upgrade. |
djahandarie
previously approved these changes
Feb 15, 2024
There was a problem hiding this comment.
LGTM. Let's think about how to improve performance in further PRs. Though I think we need to make sure we have the right set of benchmarks that really indicate end-to-end performance (esp when there are a lot of dictionaries loaded) rather than just microoptimizing the existing benchmarks we have.
I will merge after @toasted-nutbread's final review
toasted-nutbread
approved these changes
Feb 16, 2024
djahandarie
approved these changes
Feb 17, 2024
github-merge-queue bot
pushed a commit
that referenced
this pull request
Feb 17, 2024
* Copy functions from JapaneseUtil * Remove JapaneseUtil * Update usages of JapaneseUtil functions * part1 * frotend done? * fix tests * offscreen and type complications * add tests * start fixing tests * keep fixing tests * fix tests * Copy functions from JapaneseUtil * Remove JapaneseUtil * Update usages of JapaneseUtil functions * delete pt * renames * add tests * kebab-case filenames * lint * minor fixes * merge * fixes * fix part of comments * fix more comments * delete unused types * comment * comment * do backend * other files * move fetch utils to own file * remove extra line * add extra line * remove unnecessary export * simplify folder structure * remove redundant async * fix param type in api * fix language index * undo changes to cssStyleApplier * undo changes to utilities.js * undo changes to utilities.js * simplify language util * lint * undo phantom changes to anki integration * require textTransformations options * explicit locale in localeCompare * punctuate notes * prefer early exit * rename LanguageOptionsObjectMap * rename to textPreprocessor * tuple with names instead of boolean array * safe data setting * optional chaining * simplify LanguageOptions * encapsulate languages * delete language util * nullable language in text preprocessors controller * rename transform to process * remove settings * make translation advanced again * remove unused getTextTransformations api call * comments * change language types * RIP flags * comments * fix tests * lint * Text preprocessor type changes (FooSoft#10) * Add types * Update types * Simplify type check * Refactor typing and structuring of language definitions * lint * update translator benchmark * undo markdown changes * undo markdown changes * undo markdown changes * more merge * simplify language controller --------- Co-authored-by: toasted-nutbread <[email protected]> Co-authored-by: Darius Jahandarie <[email protected]>
github-merge-queue
bot
removed this pull request from the merge queue due to no response for status checks
djahandarie
added
the
kind/enhancement
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds a language select to Options > General, and makes text transformations language-specific.
I don't fully understand the offscreen proxy stuff, but i think it was necessary. The LanguageUtil on manifest v2 dynamically loaded files the first time a language was called for. Seems modules can't be loaded dynamically in the "backend" on MV3, so any suggestions to that are welcome, and as always to the naming/types.
Part 6 of #422. After this, most Yezichak users should be able to move to Yomitan.