Add glossary option #3

Closed
Androz2091 wants to merge 2 commits from Androz2091/deepl-scraper:androz2091-glossary into master
First-time contributor

Add a new glossary parameter, so I can use:

translate('You should not send DM to everyone or spam the channels', 'en', 'fr-FR', {
	'DM': 'MP',
	'channel': 'salon'
});

// will return

// Vous ne devez pas envoyer de MP à tout le monde ni spammer les salons

// instead of

// Vous ne devez pas envoyer de DM à tout le monde ou spammer les canaux.
Add a new glossary parameter, so I can use: ```js translate('You should not send DM to everyone or spam the channels', 'en', 'fr-FR', { 'DM': 'MP', 'channel': 'salon' }); // will return // Vous ne devez pas envoyer de MP à tout le monde ni spammer les salons // instead of // Vous ne devez pas envoyer de DM à tout le monde ou spammer les canaux. ```
Androz2091 added 1 commit 2021-02-16 18:59:12 +01:00
Androz2091 added 1 commit 2021-02-16 19:02:02 +01:00
Owner

As always, thank you, for taking time to contribute to my projects.

There are, however, some issues. xD

Glossary is persistent

Allowing to set glossary as a third parameter of the translate method, leads us to believe that glossary, once set, is only used for the current translation task, then forgotten, which is not the case.

Example :

const { translate } = require('deepl-scraper');
await translate('You should not send DM to everyone or spam the channels', 'en', 'fr-FR');
// -> 'Vous ne devez pas envoyer de DM à tout le monde ou spammer les canaux'
await translate('You should not send DM to everyone or spam the channels', 'en', 'fr-FR', { 'DM': 'MP', 'channel': 'salon' });
// -> 'Vous ne devez pas envoyer de MP à tout le monde ou spammer les salons'
await translate('You should not send DM to everyone or spam the channels', 'en', 'fr-FR');
// -> 'Vous ne devez pas envoyer de MP à tout le monde ou spammer les salons'
  • First result is expected : no glossary set.
  • Second result is expected : glossary is set.
  • Third result is unexpected : glossary is still set.

A more accurate implementation would consist in creating a setGlossary method, leading us to believe that, glossary, is, in fact, a persistent feature.

As the UI allows to silently replace an existing glossary entry when it is duplicated, this behavior should also be documented.

As I noticed that glossary is cleared once the quit method is called, that fact should also be documented.

Additionally, I would be most grateful, although I don't require it, if you could implement the following methods :

  • clearGlossary to remove all existing glossary entries
  • removeGlossary to remove one or many existing entries
  • getGlossary to get all existing entries

As you know, moving glossary to own methods, implies setting origin and target languages later, so you'll need to add those as second and third parameter of all glossary methods you shall implement.

Glossary doesn't support all translation-supported languages

Glossary is currently limited to English, French, German and Spanish.

const { translate } = require('deepl-scraper');
await translate('You should not send DM to everyone or spam the channels', 'en', 'it-IT', { 'DM': 'messagio' });
// -> Non dovresti inviare DM a tutti o spammare i canali

Therefore, UNSUPPORTED_SOURCE_LANGUAGE and UNSUPPORTED_TARGET_LANGUAGE must be implemented, in a dynamic way, as is the translate method.

Optionally, the getSupportedGlossaryLanguages can be implemented.


Thanks again for your involvement in this project ! :)

As always, thank you, for taking time to contribute to my projects. There are, however, some issues. xD ### Glossary is persistent Allowing to set `glossary` as a third parameter of the `translate` method, leads us to believe that glossary, once set, is only used for the current translation task, then forgotten, which is not the case. Example : ```js const { translate } = require('deepl-scraper'); await translate('You should not send DM to everyone or spam the channels', 'en', 'fr-FR'); // -> 'Vous ne devez pas envoyer de DM à tout le monde ou spammer les canaux' await translate('You should not send DM to everyone or spam the channels', 'en', 'fr-FR', { 'DM': 'MP', 'channel': 'salon' }); // -> 'Vous ne devez pas envoyer de MP à tout le monde ou spammer les salons' await translate('You should not send DM to everyone or spam the channels', 'en', 'fr-FR'); // -> 'Vous ne devez pas envoyer de MP à tout le monde ou spammer les salons' ``` - First result is expected : no glossary set. - Second result is expected : glossary is set. - Third result is unexpected : glossary is still set. A more accurate implementation would consist in creating a `setGlossary` method, leading us to believe that, glossary, is, in fact, a persistent feature. As the UI allows to silently replace an existing glossary entry when it is duplicated, this behavior should also be documented. As I noticed that glossary is cleared once the `quit` method is called, that fact should also be documented. Additionally, I would be most grateful, although I don't require it, if you could implement the following methods : - `clearGlossary` to remove all existing glossary entries - `removeGlossary` to remove one or many existing entries - `getGlossary` to get all existing entries As you know, moving glossary to own methods, implies setting origin and target languages later, so you'll need to add those as second and third parameter of all glossary methods you shall implement. ### Glossary doesn't support all translation-supported languages Glossary is currently limited to English, French, German and Spanish. ```js const { translate } = require('deepl-scraper'); await translate('You should not send DM to everyone or spam the channels', 'en', 'it-IT', { 'DM': 'messagio' }); // -> Non dovresti inviare DM a tutti o spammare i canali ``` Therefore, `UNSUPPORTED_SOURCE_LANGUAGE` and `UNSUPPORTED_TARGET_LANGUAGE` must be implemented, in a dynamic way, as is the `translate` method. Optionally, the `getSupportedGlossaryLanguages` can be implemented. --- Thanks again for your involvement in this project ! :)
Author
First-time contributor

Superseded by #4

Superseded by #4
Androz2091 closed this pull request 2021-02-20 19:15:28 +01:00

Pull request closed

Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: KaKi87/deepl-scraper#3
No description provided.