ropensci-review-tools / babeldown

Helpers for Automatic Translation of Markdown-based Content
https://docs.ropensci.org/babeldown/
Other
21 stars 2 forks source link

Update translate.R #61

Closed xtimbeau closed 7 months ago

xtimbeau commented 8 months ago

Fix #60

Hi, I made small modification to translate.R in order to protect equations form deepL. This could be set as an option, but the current behaviour of deepL on equations is destructive. I have used protect_math() for inline equations (same as protect curlies) and used digest (as for hugo shortcode) for $$ $$ separated equation.

xtimbeau commented 8 months ago

there is still a bug, I'll try to find out

maelle commented 8 months ago

@xtimbeau I edited your first comment in this PR to link the PR to your issue, see https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword

maelle commented 8 months ago

one last comment for today: when a PR isn't ready yet you can set it to "draft status" by clicking on the link at the top right, see screenshot below

image

Then you can mark it as ready for review again, but that time the button is at the bottom. https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-stage-of-a-pull-request

xtimbeau commented 8 months ago

ok nice

Le jeu. 28 mars 2024 à 11:23, Maëlle Salmon @.***> a écrit :

one last comment for today: when a PR isn't ready yet you can set it to "draft status" by clicking on the link at the top right, see screenshot below

image.png (view on web) https://github.com/ropensci-review-tools/babeldown/assets/8360597/53f39c36-7a0d-47d0-b4c4-72931a55653b

Then you can mark it as ready for review again, but that time the button is at the bottom. https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-stage-of-a-pull-request

— Reply to this email directly, view it on GitHub https://github.com/ropensci-review-tools/babeldown/pull/61#issuecomment-2024855581, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANA2KEPYSKELVP3MQLD5EIDY2PORTAVCNFSM6AAAAABFKQYOW6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRUHA2TKNJYGE . You are receiving this because you were mentioned.Message ID: @.***>

xtimbeau commented 8 months ago

Hi @maelle, finally I pinned my problem. I think it is a more general issue (and it is not related to what I have added to the code). If you prefer me to put it in a separate issue, just says so.

That trigger an error:

---
title: "Dollar"
description: "un bug dans babeldown ?"
---
une équation en ligne : $a = b + c$ est ok

mais $a$ une équation à un caractère non !

The error is:

Error in `purrr::map()` at babeldown/R/translate.R:139:3:
ℹ In index: 1.
ℹ With name: 1.
Caused by error in `char[[1]]`:
! indice hors limites

without the last line on the exemple, it works well. The problem occurs everytime you use $[.]$ or so. I can work out something to deal with that, protecting single character equation in a way or another, but may be there is something more general to do.

xtimbeau commented 8 months ago

hi @maelle, I found a workaround for the one char bug. I have another problem with multiple subscript commands in latex inline equation such as $i_t = j_t$. I have seen that there is some mention of that in tinkr https://github.com/ropensci/tinkr/issues/38 but I can't find a way to solve the issue. A way would be to protect equations like hugo shortcodes which can be done, but it seems to me the solution is already there. If you can point me to the solution, that would be great (making some progress in xml2 and so, that is very powerful).

xtimbeau commented 8 months ago

I think it is working now. The workaround is to neutralize the _. As it is set back at the end of the translation and because it doesnt change a lot of thing ofr the translation, I think it ok. It is passing the test I have reduced in work/dollar.qmd so it can be used in a test. It is possible to set an option protect_math=TRUE in the function if you like to be able to revert to previous functionning.

maelle commented 7 months ago

Thank you! Could you please remove unrelated files from the PR to make it easier to review?

maelle commented 7 months ago

I'll make a new PR using your examples, for you to review (with acknowledgement of your contribution!)

maelle commented 7 months ago

Regarding the power of XML, please allow me to share https://masalmon.eu/2022/04/08/xml-xpath/ :smile_cat:

xtimbeau commented 7 months ago

OK, fair enough. I installed tinkr from your branch, one char equation is working. I'll post a example for multiple "_"

Le ven. 5 avr. 2024 à 11:41, Maëlle Salmon @.***> a écrit :

@.**** commented on this pull request.

In R/translate.R https://github.com/ropensci-review-tools/babeldown/pull/61#discussion_r1553276931 :

temp_markdown_file <- withr::local_tempfile() markdown_lines <- brio::read_lines(path)

  • temp_markdown_file <- withr::local_tempfile()
  • change on char equation to something longer and

  • _ (used in latex) to something that is not going to be

  • markdown_lines <- gsub("\$(.)\$", "\$xXx\1\$", markdown_lines)
  • _ is a special character in markdown and in latex, we neutralize it

  • markdownlines <- gsub("", "°°", markdown_lines)

I prefer to handle such cases with XML as DeepL API can make some characters disappear (as used to happen with the ":" of footnotes 😅)

— Reply to this email directly, view it on GitHub https://github.com/ropensci-review-tools/babeldown/pull/61#pullrequestreview-1982494052, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANA2KEPOSWTT4CUGVEEVMCTY3ZWVHAVCNFSM6AAAAABFKQYOW6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTSOBSGQ4TIMBVGI . You are receiving this because you were mentioned.Message ID: @.***>

maelle commented 7 months ago

Then I can complement #67 :smile_cat: