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

Minor improvements to Quotation pure content #3000

Merged
merged 5 commits into from
Sep 24, 2023

Conversation

dato
Copy link
Contributor

@dato dato commented Sep 17, 2023

  • 25fd727 pure_content() refactor: shorter conditionals
  • 1322a0c Substitute “p.” for “page” in page progress serialization
  • ce3885d Use endposition when serializing Quotation
  • cc05cab Note content: use italics for book titles + em-dash for Quotation
  • fadf30b Also use italics for book title in editions.html template

@dato
Copy link
Contributor Author

dato commented Sep 17, 2023

Also, while we’re on this code, any reason why the title is wrapped in quotes, instead of <i>?

The quotes seem fine for a plaintext serialization, but since I see <p> and <a>, I wondered if there was a particular reason, or if I could just change it.

@mouse-reeve
Copy link
Member

No reason I can think of, as long as it's in the content of the serialization rather than an Article title, which it shuldn't be since this is a Note not an Article. You're welcome to change it. Chances are, at the time I simply didn't realize that Mastodon would render italics (but it does so that's a non-issue)

@dato
Copy link
Contributor Author

dato commented Sep 17, 2023

Thanks, will do and ask for re-review.

@dato dato marked this pull request as draft September 17, 2023 13:50
@dato dato force-pushed the position_serialization branch from 667c3a5 to ce3885d Compare September 17, 2023 18:21
@dato dato changed the title Minor improvements to page serialization in Quotation Minor improvements to Quotation pure content Sep 17, 2023
@dato dato marked this pull request as ready for review September 17, 2023 19:13
@dato
Copy link
Contributor Author

dato commented Sep 17, 2023

Done. I placed each item in a separate commit.

dato added a commit to dato/bookwyrm that referenced this pull request Sep 18, 2023
@dato dato mentioned this pull request Sep 18, 2023
@mouse-reeve
Copy link
Member

This looks good, can you run the Python formatter? With that, I will be ready to merge

@dato dato force-pushed the position_serialization branch from 8d1d810 to fadf30b Compare September 23, 2023 20:50
@dato
Copy link
Contributor Author

dato commented Sep 23, 2023

Oops, my bad. Fixed.

@mouse-reeve mouse-reeve merged commit bab28a8 into bookwyrm-social:main Sep 24, 2023
10 checks passed
@dato dato deleted the position_serialization branch September 25, 2023 16:26
dato added a commit to dato/bookwyrm that referenced this pull request Sep 25, 2023
Sadly, this pulls other unrelated pull requests, because I didn't
base it on v0.6.5

Replaces:

    commit bab28a8

    Merge pull request bookwyrm-social#3000 from dato/position_serialization

    Minor improvements to Quotation pure content
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.

2 participants