Skip to content

Migrate to typescript, vite and vitest #28

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AntonOfTheWoods
Copy link

I have been experimenting with VSCode's new Agent Mode... I realise there is not a huge benefit to this as the current code works where it should, but it might make it easier/more appearing to add functionality.

Copy link
Member

@graphemecluster graphemecluster left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution. We will review them further later.

Comment on lines -82 to +125
['', '「'],
['', '」'],
['', '『'],
['', '』'],
['"', '「'],
['"', '」'],
[''', '『'],
[''', '』'],
];
const converter = OpenCC.ConverterFactory(
OpenCC.Locale.from.cn, // Simplified Chinese (Mainland China) => OpenCC standard
OpenCC.Locale.to.tw.concat([customDict]) // OpenCC standard => Traditional Chinese (Taiwan) with custom words
);
console.log(converter('悟空道:师父又来了。怎么叫做水中捞月’?”'));
console.log(converter('悟空道:"师父又来了。怎么叫做'水中捞月'?"'));
// output: 悟空道:「師父又來了。怎麼叫做『水中撈月』?」
```

This will get the same result with an extra convertion.

```javascript
const customDict = [
['', '「'],
['', '」'],
['', '『'],
['', '』'],
['"', '「'],
['"', '」'],
[''', '『'],
[''', '』'],
];
const converter = OpenCC.ConverterFactory(
OpenCC.Locale.from.cn, // Simplified Chinese (Mainland China) => OpenCC standard
OpenCC.Locale.to.tw, // OpenCC standard => Traditional Chinese (Taiwan)
[customDict] // Traditional Chinese (Taiwan) => custom words
);
console.log(converter('悟空道:师父又来了。怎么叫做水中捞月’?”'));
console.log(converter('悟空道:"师父又来了。怎么叫做'水中捞月'?"'));
Copy link
Member

Choose a reason for hiding this comment

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

What happened to these lines? ''' is definitely incorrect JavaScript.

Copy link
Author

Choose a reason for hiding this comment

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

One thing VSCode's agent mode is completely useless at is quotes - it simply doesn't consider them to be different!

Copy link
Collaborator

Choose a reason for hiding this comment

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

The original example considered a scenario found in some web novels, where punctuation conventions differ between CN and TW. Therefore, it provided a way for users to customize replacement terms.

In addition, if you map " to both and , as in lines 115 and 116, the latter will override the former.

So I don't understand the purpose of this change.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry, please ignore this precise change for the moment. As I mentioned above, this was a broken automated change that I didn't fix. This is just a draft PR to get "in principle" feedback as to whether a migration to TS might get merged. Some people hate TS, and would never accept it... So I didn't want to waste time perfecting something that would never be accepted...

Copy link
Collaborator

@ren1244 ren1244 Apr 22, 2025

Choose a reason for hiding this comment

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

I hope my previous comment didn’t make you feel uncomfortable. I personally have no opinion on whether to use TypeScript; I'm only responding regarding this specific file.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think this file is useful on its own without further descriptions

@graphemecluster
Copy link
Member

Would you also like to work on the CI workflows?

@AntonOfTheWoods
Copy link
Author

Would you also like to work on the CI workflows?

Sure. Basically, if you think it is worth migrating to typescript, I'll put in quite a bit of effort, including CI.

This was just a first pass that I wanted to touch base on - there is some cruft from the automated translation that needs to be cleaned/fixed. I'm busy with other stuff for the next few days but will circle back soon, as long as you are interested in the basic principle of migrating to TS.

@graphemecluster
Copy link
Member

I personally love TypeScript very much! But let me seek other members’ opinions first.

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.

3 participants