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

Update the rules for parsing subjects #125

Merged
merged 17 commits into from
May 15, 2020
Merged

Update the rules for parsing subjects #125

merged 17 commits into from
May 15, 2020

Conversation

qnga
Copy link
Contributor

@qnga qnga commented Mar 21, 2020

  • Add rules for sortAs, code and scheme properties
  • Don't try to split subjects if they are refined by some meta element
  • Make the parsing of subjects compliant with the rule

when parsing a publication in the streamer we always use the most complex form for each metadata to harmonize our output.

Fix #111.

@HadrienGardeur
Copy link
Contributor

This should be reviewed by @JayPanoz since he wrote the initial document.

JayPanoz
JayPanoz reviewed Mar 22, 2020
View reviewed changes
streamer/parser/metadata.md Outdated Show resolved Hide resolved
streamer/parser/metadata.md Outdated Show resolved Hide resolved
streamer/parser/metadata.md Outdated Show resolved Hide resolved
streamer/parser/metadata.md Outdated Show resolved Hide resolved
streamer/parser/metadata.md Outdated Show resolved Hide resolved
streamer/parser/metadata.md Outdated Show resolved Hide resolved
streamer/parser/metadata.md Outdated Show resolved Hide resolved
streamer/parser/metadata.md Outdated Show resolved Hide resolved
@qnga
Copy link
Contributor Author

qnga commented Mar 22, 2020

Thanks for the improvements JayPanoz.

JayPanoz
JayPanoz requested changes Mar 23, 2020
View reviewed changes
streamer/parser/metadata.md Outdated Show resolved Hide resolved
streamer/parser/metadata.md Outdated Show resolved Hide resolved
@JayPanoz JayPanoz mentioned this pull request Mar 23, 2020
Closed
JayPanoz
JayPanoz requested changes Mar 24, 2020
View reviewed changes
streamer/parser/metadata.md Outdated Show resolved Hide resolved
streamer/parser/metadata.md Outdated Show resolved Hide resolved
streamer/parser/metadata.md Outdated Show resolved Hide resolved
streamer/parser/metadata.md Outdated Show resolved Hide resolved
JayPanoz
JayPanoz requested changes Mar 24, 2020
View reviewed changes
Copy link
Contributor

@JayPanoz JayPanoz left a comment

Choose a reason for hiding this comment

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

On second thought, fine-tuned the “case of conflict” paragraph as it makes the logic for the mapping tables more explicit, and more feature-proof – on top of covering cases we might have forgotten.

Also removed <dc:publisher> from this paragraph for EPUB conformance.

I think we’ll then be good to go. Thanks for the PR!

streamer/parser/metadata.md Outdated Show resolved Hide resolved
streamer/parser/metadata.md Outdated Show resolved Hide resolved
JayPanoz
JayPanoz approved these changes Mar 25, 2020
View reviewed changes
Copy link
Contributor

@JayPanoz JayPanoz left a comment

Choose a reason for hiding this comment

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

LGTM.

Thanks for the improvements!

@qnga qnga mentioned this pull request Mar 25, 2020
Closed
@JayPanoz JayPanoz mentioned this pull request Apr 30, 2020
Merged
qnga and others added 17 commits May 1, 2020 11:21
@qnga
Fix a typo
d5cf242
@qnga
Fix inheritance rules for xml:lang
c0779fb
@qnga
Clarify how language maps should be computed.
4f748ee
@qnga
Precise rules for determining contributor's key
605a511
@qnga
Update the rules for parsing subjects
7ef0d76 
Fix readium#111.
@qnga @JayPanoz
Update streamer/parser/metadata.md
b809382 
Co-Authored-By: Jiminy Panoz <JayPanoz@users.noreply.github.com>
@qnga @JayPanoz
Update streamer/parser/metadata.md
1f23ad2 
Co-Authored-By: Jiminy Panoz <JayPanoz@users.noreply.github.com>
@qnga @JayPanoz
Update streamer/parser/metadata.md
60592ca 
Co-Authored-By: Jiminy Panoz <JayPanoz@users.noreply.github.com>
@qnga @JayPanoz
Fix apostrophes
2d1d56e 
Co-Authored-By: Jiminy Panoz <JayPanoz@users.noreply.github.com>
@qnga
Fix a typo
5d7f487
@qnga @JayPanoz
Apply suggestions from code review
1eeb428 
Co-Authored-By: Jiminy Panoz <JayPanoz@users.noreply.github.com>
@qnga @JayPanoz
Improve the table for contributor parsing
b650382 
Close readium#126

Co-Authored-By: Jiminy Panoz <JayPanoz@users.noreply.github.com>
@qnga
Remove the paragraph about role overriding
05739ee
@qnga
Change the order in the contributor table and make explicit cases whe…
e6045ca 
…n role is not applicable
@qnga
Remove fallback to dc:language for metadata language
8600474
@qnga
Update file-as parsing so that sortAs is a localized string
e9ba515
@qnga
Add fallbacks to Epub 2 case for series and title's sorting key
776ea1d
@JayPanoz
Copy link
Contributor

JayPanoz commented May 1, 2020

I guess this PR + #135 can be merged now.

Next step for these EPUB parsing docs will be, as discussed a month ago during the engineering call, re-organising the EPUB doc a little bit based on nav doc (landmarks), opf, etc.

@mickael-menu
Copy link
Member

I'll merge it this Friday, unless someone vetoes it first.

@mickael-menu mickael-menu merged commit f53cbee into readium:master May 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JayPanoz JayPanoz JayPanoz approved these changes

@HadrienGardeur HadrienGardeur HadrienGardeur left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Parsing Epub subjects
4 participants
@qnga @HadrienGardeur @JayPanoz @mickael-menu