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

[Flashcards/KELLIA] Content is Dependent on the XML Wrapping Width #145

Open
pishoyg opened this issue Aug 6, 2024 · 4 comments
Open

[Flashcards/KELLIA] Content is Dependent on the XML Wrapping Width #145

pishoyg opened this issue Aug 6, 2024 · 4 comments
Labels
dev Why: Developer experience UI Why: Better user interface user Why: User convenience

Comments

@pishoyg
Copy link
Owner

pishoyg commented Aug 6, 2024

Bug discovered in 6279509. Reproducing the commit message:

[Flashcards/KELLIA] Run make flashcards test to pick up changes.
The changes are supposed to affect formatting only. However, they seem
to have caused the flashcards pipeline to insert a lot of
tags
unnecessarily.

Here is how this happened:

  • We start wrapping the source XML files at 80 characters.
  • Our KELLIA binary reads them, and passes the content as is to the
    output TSV's.
  • Our Flashcards binary reads the TSV's, and replaces the newline
    characters with
    tags.

This looks like a bug that need to be resolved. Let's do it when we are
more fluent in KELLIA! (The code is mostly copied.)

#140 introduced a parameter to help solve the problem.

I believe the path forward is:

  • The KELLIA pipeline produces well-formatted HTML.
  • In the Flashcards pipeline, we do NOT convert any newlines to <br/> tags.

P.S. It's cleaner to do the above for all Flashcard pipelines.

@pishoyg pishoyg added the dev Why: Developer experience label Aug 6, 2024
pishoyg added a commit that referenced this issue Aug 6, 2024
Running `make flashcards test` does NOT introduce any changes, as
intended.

This ensures that all instances where you replace newlines with line
breaks are visible with a search for the string `line_br=True`.
@pishoyg
Copy link
Owner Author

pishoyg commented Aug 6, 2024

#146 is a more general version of this issue.

@pishoyg pishoyg added user Why: User convenience bug Why: Fix a bug and removed dev Why: Developer experience labels Aug 9, 2024
@pishoyg pishoyg added UI Why: Better user interface dev Why: Developer experience and removed bug Why: Fix a bug labels Aug 17, 2024
@pishoyg pishoyg added this to coptic Sep 11, 2024
@pishoyg
Copy link
Owner Author

pishoyg commented Oct 3, 2024

Tidy has a add-xml-space flag that could be helpful (see).

@pishoyg
Copy link
Owner Author

pishoyg commented Oct 6, 2024

NOTE: #265.

@pishoyg
Copy link
Owner Author

pishoyg commented Oct 9, 2024

This may no longer be the case. We need to verify.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev Why: Developer experience UI Why: Better user interface user Why: User convenience
Projects
Status: No status
Development

No branches or pull requests

1 participant