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

add recursive function to find currency dollars #123

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

zkamvar
Copy link
Member

@zkamvar zkamvar commented Oct 16, 2024

This adds a stronger solution to #121.

It does a second check to see if there are more opening dollar signs than
closing and then passes those through a function that walks the list of broken
math elements and detects the opening elements that aren't paired with closing
elements (by confirming that the pairs are adjacent). The opening elements that
aren't paired are excluded from the list and then if the list is still not
balanced an error is thrown.

This does not work because it breaks the postfix dollar treatment
This makes it easier to re-test once we've transformed
1. If there are no headless math, then just return because it's all money
2. If there is are more endless than headless, toss the money and keep
   going

Ultimately, if there is an imbalance (even after 2), then an error
should be thrown
The tests for the postfix dollars have now changed because we are sort
of loosening the restrictions, which ultimately is a good thing because
people will run into fewer errors when protecting math.
This simplifies the function to three conditionals from four and clearly
labels the condidtions, starting with the exit condition.
Base automatically changed from znk/money-money-2020/121 to main October 16, 2024 04:23
This was referenced Oct 16, 2024
@zkamvar zkamvar requested a review from maelle November 5, 2024 17:02
@zkamvar zkamvar closed this Nov 5, 2024
@zkamvar zkamvar deleted the znk/toss-money/121 branch November 5, 2024 17:02
@zkamvar zkamvar restored the znk/toss-money/121 branch November 5, 2024 17:02
@zkamvar zkamvar reopened this Nov 5, 2024
Copy link
Member

@maelle maelle left a comment

Choose a reason for hiding this comment

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

thank you!! another solution: use € not $ 😸

@@ -129,7 +129,7 @@ protect_inline_math <- function(body, ns) {
}

# protect math that is broken across lines or markdown elements
if (length(bmath)) {
Copy link
Member

Choose a reason for hiding this comment

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

maybe use an explaining variable although that is already clear

le <- length(bmath[endless])
lh <- length(bmath[headless])
# 2024-10-10: if the number of headless tags is zero, then we are dealing
# with currency. See issue #121
if (lh == 0) {
return(copy_xml(body))
}
# 2024-10-15: if the number of headless tags is _less_ than the number of
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# 2024-10-15: if the number of headless tags is _less_ than the number of
# 2024-10-15: if the number of startless tags is _less_ than the number of

Copy link
Member

Choose a reason for hiding this comment

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

the variable is actually headless, but I wonder if it might be confusing

}
# return the result and iterate over the rest of the vector
return(c(result, toss_broken_teeth(endless[idx], headless[idx])))
}
Copy link
Member

Choose a reason for hiding this comment

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

should we use devtag https://github.com/moodymudskipper/devtag

and have examples

tinkr::yarn$new(ex)$protect_math()$get_protected()
```
2. Postfix dollar signs (e.g. using BASIC code demostrations) should be
protected with backtics, or it will cause an error.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
protected with backtics, or it will cause an error.
protected with backticks, or it will cause an error.

ex <- textConnection("$5 for _any_ $\\pi$")
tinkr::yarn$new(ex)$protect_math()$get_protected()
```
2. Postfix dollar signs (e.g. using BASIC code demostrations) should be
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
2. Postfix dollar signs (e.g. using BASIC code demostrations) should be
2. Postfix dollar signs (e.g. using BASIC code demonstrations) should be

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