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.

Sign up for GitHub

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

Jump to bottom

fix white space #101

Merged
merged 5 commits into from
Nov 16, 2021
Merged

fix white space #101

merged 5 commits into from
Nov 16, 2021

Conversation

da1nerd
Copy link

@da1nerd da1nerd commented Sep 3, 2021 •
edited
Loading

See unfoldingWord/translationCore#7164

\p should not have any trailing spaces before the newline

I added a fix for unfoldingWord/translationCore#7165 as well since it was closely related. This removes white space after the pipe |.

There are a ton of file changes, but those are mostly updated test files.

This change is Reviewable

@da1nerd da1nerd changed the base branch from develop to master September 3, 2021 03:20
@da1nerd da1nerd self-assigned this Sep 3, 2021
@RobH123
Copy link

RobH123 commented Sep 6, 2021 •
edited
Loading

@da1nerd I noticed these warnings in a USFM check today, and wondered if they might also be covered by this set of changes:
LT Unexpected USFM field with 'line marker='\w'' at EXO 6:14 in en_ult repository in file 02-EXO.usfm on line 3,895 around ◗fathers:␣\p◖ (Priority 875)
LT Unexpected USFM field with 'line marker='\zaln-s'' at EXO 6:23 in en_ult repository in file 02-EXO.usfm on line 4,114 around ◗Ithamar.\p␣␣◖ (Priority 875)
LT Unexpected USFM field with 'line marker='\w'' at EXO 6:29 in en_ult repository in file 02-EXO.usfm on line 4,253 around ◗you.”\p␣␣◖ (Priority 875)
ST Unexpected USFM field with 'line marker='\w'' at EXO 6:14 in en_ust repository in file 02-EXO.usfm on line 4,120 around ◗leaders.\p◖ (Priority 875)
ST Unexpected USFM field with 'line marker='\w'' at EXO 6:14 in en_ust repository in file 02-EXO.usfm on line 4,140 around ◗them.\p␣␣◖ (Priority 875)
ST Unexpected USFM field with 'line marker='\w'' at EXO 6:29 in en_ust repository in file 02-EXO.usfm on line 4,429 around ◗you.”\p␣␣◖ (Priority 875)

Could it also be USFM-JS sometimes appending \p markers to the ends of lines and often with superfluous spaces before or after? (The \p markers should be on the following line.)

@da1nerd
Copy link
Author

da1nerd commented Sep 6, 2021

@RobH123 I'm making some additional tweaks to fix those bugs you mentioned.
In doing so I ended up changing some other things I'd like to run by you.

  • Is it ok to eliminate all blank lines from usfm except for a blank line at the end of the file? This would include removing blank lines before \s5 markers
  • Is it ok to eliminate all white space at the end of each line of usfm?

@RobH123
Copy link

RobH123 commented Sep 6, 2021 •
edited
Loading

Great! I'm glad I noticed those while your mind was still in here. I'd forgotten that it often did that.

No to #1. Please leave blank lines in. The editors use them for visual breaks. They do no harm.

If you wanted, I think it would be ok to normalise two or more blank lines just to one.

Please ensure that the file ends with a (single) newline.

Yes to #2. I know of no need to ever have trailing spaces.

Thanks @da1nerd .

PhotoNomad0
PhotoNomad0 requested changes Sep 7, 2021
View reviewed changes
__tests__/resources/acts-1-20.aligned.oldformat.usfm Outdated
@@ -1,86 +1,86 @@
\id ACT EN_ULT en_English_ltr Wed Oct 10 2018 11:55:23 GMT-0400 (EDT) tc
\c 1
\v 20 "
\zaln-s | x-strong="G10630" x-lemma="γάρ" x-morph="Gr,CC,,,,,,,," x-occurrence="1" x-occurrences="1" x-content="γὰρ"
\zaln-s |x-strong="G10630" x-lemma="γάρ" x-morph="Gr,CC,,,,,,,," x-occurrence="1" x-occurrences="1" x-content="γὰρ"
Copy link

Choose a reason for hiding this comment

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

We should leave the spaces in this file. This is to verify that we can still import the old file format.

__tests__/resources/acts_1_11.aligned.oldformat.usfm Outdated
@@ -1,106 +1,106 @@
\id ACT EN_ULT en_English_ltr Wed Oct 10 2018 11:55:23 GMT-0400 (EDT) tc
\c 1
\v 1
\zaln-s | x-strong="G25320" x-lemma="καί" x-morph="Gr,D,,,,,,,,," x-occurrence="1" x-occurrences="1" x-content="καὶ"
\zaln-s |x-strong="G25320" x-lemma="καί" x-morph="Gr,D,,,,,,,,," x-occurrence="1" x-occurrences="1" x-content="καὶ"
Copy link

Choose a reason for hiding this comment

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

ditto

__tests__/resources/acts_1_4.aligned.oldformat.usfm
@@ -1,69 +1,69 @@
\id ACT EN_ULT en_English_ltr Wed Oct 10 2018 11:55:23 GMT-0400 (EDT) tc
\c 1
\v 4
\zaln-s | x-lemma="καί" x-morph="Gr,CC,,,,,,,," x-occurrence="1" x-occurrences="1" x-strong="G25320" x-content="καὶ"
\zaln-s |x-lemma="καί" x-morph="Gr,CC,,,,,,,," x-occurrence="1" x-occurrences="1" x-strong="G25320" x-content="καὶ"
Copy link

Choose a reason for hiding this comment

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

ditto

@PhotoNomad0
Copy link

@da1nerd This is probably ready to go with just the above tweaks.

@PhotoNomad0 PhotoNomad0 mentioned this pull request Nov 8, 2021
Closed
PhotoNomad0
PhotoNomad0 approved these changes Nov 16, 2021
View reviewed changes
Copy link

@PhotoNomad0 PhotoNomad0 left a comment

Choose a reason for hiding this comment

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

LGTM

@PhotoNomad0 PhotoNomad0 merged commit 2e74e80 into master Nov 16, 2021
@PhotoNomad0 PhotoNomad0 deleted the bugfix-joel-7164 branch November 20, 2021 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PhotoNomad0 PhotoNomad0 PhotoNomad0 approved these changes

Assignees

@da1nerd da1nerd

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@da1nerd @RobH123 @PhotoNomad0