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

preserve whitespace in cloze-prefix and cloze-suffix #755

Merged

Conversation

StefanVukovic99
Copy link
Collaborator

Fixes #744
Basically, it seems the handlebars ~ removes whitespace but not newlines. Instead of trimming the string outputted by handlebars this just removes the newlines.

@StefanVukovic99 StefanVukovic99 requested a review from a team as a code owner March 1, 2024 22:12
Copy link

codspeed-hq bot commented Mar 1, 2024

CodSpeed Performance Report

Merging #755 will degrade performances by 19.96%

Comparing StefanVukovic99:cloze-handlebars-whitespace (275d697) with master (e47a0f4)

Summary

❌ 1 regressions
✅ 3 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark master StefanVukovic99:cloze-handlebars-whitespace Change
Translator.prototype.findTerms - (n=39) 790.9 ms 988.1 ms -19.96%

Copy link

github-actions bot commented Mar 1, 2024

✔️ No visual differences introduced by this PR.

View Playwright Report (note: open the "playwright-report" artifact)

@Kuuuube
Copy link
Member

Kuuuube commented Mar 1, 2024

Note that this will have conflicts with #733.

@Casheeew Casheeew added the kind/bug The issue or PR is regarding a bug label Mar 2, 2024
@djahandarie djahandarie added this pull request to the merge queue Mar 3, 2024
Merged via the queue into yomidevs:master with commit 5ec2344 Mar 3, 2024
10 checks passed
@Casheeew Casheeew added the area/anki The issue or PR is related to Anki integration label Mar 4, 2024
@kuroahna
Copy link

kuroahna commented Mar 4, 2024

Hi @StefanVukovic99 I think this needs a 2nd look. It's broken other handlebars templates now such as {expression} and {reading}.

In Anki Card Templates, I have

  1. Scanned text = 読め
  2. Card field = {expressino}
    And then I click Test

The card render.result returns me








読める

with a lot of newlines

Reverting the commit properly returns 読める without the newlines

@StefanVukovic99
Copy link
Collaborator Author

That is strange
Screenshot from 2024-03-04 21-15-36
Screenshot from 2024-03-04 21-15-47
Any other info? Does it do the same when you add the card to anki?

@kuroahna
Copy link

kuroahna commented Mar 4, 2024

Sorry for the noise @StefanVukovic99 and thanks for resolving it with me in a DM , I reverted to the default template and it works. I had

        {{#each glossary}}{{#formatGlossary ../dictionary}}{{#regexReplace "^[^\n]*\n" ""}}{{{.}}}{{/regexReplace}}{{/formatGlossary}}{{/each}}

in

{{#*inline "glossary-single"}}
    {{~#unless brief~}}
        {{~#scope~}}
            {{~set "any" false~}}
            {{~#each definitionTags~}}
                {{~#if (op "||" (op "!" @root.compactTags) (op "!" redundant))~}}
                    {{~#if (get "any")}}, {{else}}<i>({{/if~}}
                    {{name}}
                    {{~set "any" true~}}
                {{~/if~}}
            {{~/each~}}
            {{~#unless noDictionaryTag~}}
                {{~#if (op "||" (op "!" @root.compactTags) (op "!==" dictionary (get "previousDictionary")))~}}
                    {{~#if (get "any")}}, {{else}}<i>({{/if~}}
                    {{dictionary}}
                    {{~set "any" true~}}
                {{~/if~}}
            {{~/unless~}}
            {{~#if (get "any")}})</i> {{/if~}}
        {{~/scope~}}
        {{~#if only~}}({{#each only}}{{.}}{{#unless @last}}, {{/unless}}{{/each}} only) {{/if~}}
    {{~/unless~}}
    {{~#if (op "<=" glossary.length 1)~}}
        {{#each glossary}}{{#formatGlossary ../dictionary}}{{#regexReplace "^[^\n]*\n" ""}}{{{.}}}{{/regexReplace}}{{/formatGlossary}}{{/each}}
    {{~else if @root.compactGlossaries~}}
        {{#each glossary}}{{#formatGlossary ../dictionary}}{{#regexReplace "^[^\n]*\n" ""}}{{{.}}}{{/regexReplace}}{{/formatGlossary}}{{#unless @last}} | {{/unless}}{{/each}}
    {{~else~}}
        <ul>{{#each glossary}}<li>{{formatGlossary ../dictionary .}}</li>{{/each}}</ul>
    {{~/if~}}
    {{~set "previousDictionary" dictionary~}}
{{/inline}}

from an old handlebars template I copied.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/anki The issue or PR is related to Anki integration kind/bug The issue or PR is regarding a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cloze-prefix and cloze-suffix not preserving whitespace
6 participants