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

Handle newlines in tag editing #6354

Merged

Conversation

mheiman
Copy link
Collaborator

@mheiman mheiman commented Mar 31, 2022

Closes #6346

Fixes the problem of line breaks in tag fields (subjects/places/times/people) causing items after the line break to be lost.

Technical

I had hoped that the csv.py library being used to parse tag data would have a configuration option to ignore line breaks, but the documentation indicates that the reader() method explicitly ignores attempts to redefine line breaks (which is what caused the bug in the first place).

Failing that, the best solution seemed to be to just strip out any line breaks immediately before parsing. I tested on both Mac and Windows to verify that the form is sending \r\n regardless of platform.

Testing

Edit a work subject list, adding a line break between entries. They should now all save correctly.

Stakeholders

@seabelis @cdrini @mekarpeles

Attribution Disclaimer: By proposing this pull request, I affirm to have made a best-effort and exercised my discretion to make sure relevant sections of this code which substantially leverage code suggestions, code generation, or code snippets from sources (e.g. Stack Overflow, GitHub) have been annotated with basic attribution so reviewers & contributors may have confidence and access to the correct context to evaluate and use this code.

@mekarpeles mekarpeles self-assigned this Mar 31, 2022
@mekarpeles mekarpeles added the On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing label Mar 31, 2022
@mekarpeles
Copy link
Member

thank you @mheiman. Adding to testing so we can try the fix out. Otherwise, code lgtm

@mekarpeles mekarpeles merged commit 01704ff into internetarchive:master Apr 1, 2022
@jimchamp jimchamp removed the On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing label Apr 2, 2024
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.

Tags are deleted/lost with line break
3 participants