-
Notifications
You must be signed in to change notification settings - Fork 7
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
Custom dictionary #34
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, I like this idea. I have trouble with a few words myself. But, I need to test it and would like you to consider my feedback.
customDictionary.ts
Outdated
// Replace text with custom dictionary entries | ||
export function replaceTextWithCustomDictionary(text: string, customDictionary: CustomDictionarySettings['customDictionary']): string { | ||
for (const entry of customDictionary) { | ||
text = text.replace(new RegExp(entry.source, 'g'), entry.replace); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hrm. I am curious of two things:
-
Might it be better to use the strategy here to avoid looping multiple times over the text?
-
Would it be better to use a word boundary
\b
to ensure you don't replace things in the middle of words?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@philips Thanks for feedback. I took your feedback use the faster matching strategy. Regarding the work boundary, I often use acronyms or special symbols in my notes that often get broken up as multiple words. To optimize for individual workflows, I added two options to each dictionary entry: case insensitive matching and word boundary matching.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@philips, have you had a chance to test this? I've been using it on my fork for about a month without issues. It would be great to get this merged for others to use.
Adds a settings panel to configure a custom dictionary: #32.