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

Expand documentation on valid text files #176

Merged
merged 5 commits into from
Oct 10, 2023
Merged

Conversation

Enkidu93
Copy link
Collaborator

@Enkidu93 Enkidu93 commented Oct 10, 2023

Note: This was part of earlier work done when addressing the issue of strict parsing before it was decided that Serval should not validate files. It would still be beneficial to incorporate.


This change is Reviewable

Note: This was part of earlier work done when addressing the issue of strict parsing before it was decided that Serval should not validate files. It would still be beneficial to incorporate.
@Enkidu93 Enkidu93 requested a review from johnml1135 October 10, 2023 17:54
Copy link
Contributor

@ddaspit ddaspit left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Enkidu93 and @johnml1135)


src/Serval.DataFiles/Controllers/DataFilesController.cs line 97 at r1 (raw file):

    ///     > verse_001_006	(tab) Ἀγγέλους τε τοὺς μὴ τηρήσαντας τὴν ἑαυτῶν ἀρχήν , ἀλλὰ (tab) ss
    ///     > verse_001_007 (tab) Ὡς Σόδομα καὶ Γόμορρα , καὶ αἱ περὶ αὐτὰς πόλεις (tab) ss
    ///   * Otherwise, *no tabs* should be used in the file and a unique identifier will generated for each translation unit based on the linenumber.

I think you missed a space in linenumber.

@Enkidu93
Copy link
Collaborator Author

src/Serval.DataFiles/Controllers/DataFilesController.cs line 97 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

I think you missed a space in linenumber.

Oh, thank you! Weirdly spellcheck did not catch that.

@Enkidu93
Copy link
Collaborator Author

src/Serval.DataFiles/Controllers/DataFilesController.cs line 97 at r1 (raw file):

Previously, Enkidu93 (Eli C. Lowry) wrote…

Oh, thank you! Weirdly spellcheck did not catch that.

Done.

@johnml1135
Copy link
Collaborator

src/Serval.DataFiles/Controllers/DataFilesController.cs line 95 at r2 (raw file):

    ///   * If a line contains a tab, characters before the tab are used as a unique identifier for the line, characters after the tab are understood as the content of the verse, and if there is another tab following the verse content, characters after this second tab are assumed to be column codes like "ss" etc. for sectioning and other formatting. See this example of a tab-delimited text file:
    ///     > verse_001_005 (tab) Ὑπομνῆσαι δὲ ὑμᾶς βούλομαι , εἰδότας ὑμᾶς ἅπαξ τοῦτο
    ///     > verse_001_006	(tab) Ἀγγέλους τε τοὺς μὴ τηρήσαντας τὴν ἑαυτῶν ἀρχήν , ἀλλὰ (tab) ss

there actually is a tab in "verse_001_006 (tab)". that should be replaced with a space.

@Enkidu93
Copy link
Collaborator Author

src/Serval.DataFiles/Controllers/DataFilesController.cs line 95 at r2 (raw file):

Previously, johnml1135 (John Lambert) wrote…

there actually is a tab in "verse_001_006 (tab)". that should be replaced with a space.

Thank you! Done.

@johnml1135
Copy link
Collaborator

src/Serval.DataFiles/Controllers/DataFilesController.cs line 98 at r2 (raw file):

    ///     > verse_001_007 (tab) Ὡς Σόδομα καὶ Γόμορρα , καὶ αἱ περὶ αὐτὰς πόλεις (tab) ss
    ///   * Otherwise, *no tabs* should be used in the file and a unique identifier will generated for each translation unit based on the line number.
    /// * **Paratext**: A complete, zipped Paratext project

There is little to no documentation online as to how to backup and restore a paratext project - apart from a few old pdf's made of older versions. All the advice is to not use backup/restore but rather use the paratext send/receive server. I would be inclined to either leave the enhanced documentation here, or refer to a specific url (if we can find one).

Copy link
Collaborator

@johnml1135 johnml1135 left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 2 files at r1, 1 of 1 files at r2.
Reviewable status: 1 of 2 files reviewed, 3 unresolved discussions (waiting on @ddaspit and @Enkidu93)

Copy link
Collaborator

@johnml1135 johnml1135 left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ddaspit and @Enkidu93)

@codecov-commenter
Copy link

codecov-commenter commented Oct 10, 2023

Codecov Report

All modified lines are covered by tests ✅

Files Coverage Δ
src/Serval.Client/Client.g.cs 72.55% <100.00%> (ø)
...erval.DataFiles/Controllers/DataFilesController.cs 96.87% <ø> (ø)

📢 Thoughts on this report? Let us know!.

@Enkidu93
Copy link
Collaborator Author

src/Serval.DataFiles/Controllers/DataFilesController.cs line 98 at r2 (raw file):

Previously, johnml1135 (John Lambert) wrote…

There is little to no documentation online as to how to backup and restore a paratext project - apart from a few old pdf's made of older versions. All the advice is to not use backup/restore but rather use the paratext send/receive server. I would be inclined to either leave the enhanced documentation here, or refer to a specific url (if we can find one).

Good catch. I've put it back. That was just a merge error.

Copy link
Collaborator

@johnml1135 johnml1135 left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ddaspit)

Copy link
Collaborator

@johnml1135 johnml1135 left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ddaspit)

Copy link
Contributor

@ddaspit ddaspit left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 2 files at r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Enkidu93)

@johnml1135 johnml1135 merged commit 0f6a78e into main Oct 10, 2023
1 check passed
@ddaspit ddaspit deleted the serval_datafile_doc_update branch October 13, 2023 19:58
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.

4 participants