Skip to content
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

Remove default italic #1654

Closed
wants to merge 12 commits into from
Closed

Remove default italic #1654

wants to merge 12 commits into from

Conversation

BoboTiG
Copy link
Owner

@BoboTiG BoboTiG commented Jan 22, 2023

It will help us catch missing templates without --check-word.

@BoboTiG BoboTiG requested a review from lasconic January 22, 2023 05:25
@sourcery-ai

This comment was marked as spam.

@BoboTiG
Copy link
Owner Author

BoboTiG commented Jan 22, 2023

That's a small change but with lots of consequences ^^

Running --render on EN, and FR, outputs lot more missing templates. But some are real issues (templates not well handled because of automatic italic/capitalize).

It will not ease our work at first, but then our rendering will be more identic to the Wiktionary itself.

WDYT?

@lasconic
Copy link
Collaborator

Maybe better as a second pass ? Once we have stabilize here https://github.com/BoboTiG/ebook-reader-dict/blob/master/wikidict/lang/defaults.py#L102 we can go our way up in this function ? Or you prefer to have all of them at once ? Could be a bit discouraging ?

@lasconic
Copy link
Collaborator

For reference, I catched > 150 not supported templates in english. Most of them used 1 or 2 times in the whole dictionary... Already quite a lot of work.

@BoboTiG
Copy link
Owner Author

BoboTiG commented Jan 22, 2023

Could be a bit discouraging ?

Clearly :)

Let's delay such change, you 're right, we already have soo much to do.

@BoboTiG
Copy link
Owner Author

BoboTiG commented Jan 22, 2023

For reference, I catched > 150 not supported templates in english. Most of them used 1 or 2 times in the whole dictionary... Already quite a lot of work.

Also, if you prefer to commit big changes in one commit, go ahead. Creating issues is time consuming.

@@ -78,10 +78,6 @@ def last_template_handler(
text = parts[1] if len(parts) == 2 else word
return transliterate(lang, text)

# {{tpl|item}} -> <i>(Templatet gf)</i>
if len(template) == 2:
return term(capitalize(lookup_italic(tpl, locale)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe the best way would be to call lookup_italic(tpl, locale, True): here, just like above.
Then the template will be reported by the MISSING_TPL_SEEN code.

@BoboTiG BoboTiG marked this pull request as draft September 22, 2024 06:48
@BoboTiG BoboTiG closed this Oct 22, 2024
@BoboTiG BoboTiG deleted the fix-default-italic branch October 22, 2024 20:29
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.

2 participants