GetRD / academic-file-converter

📚 Import Bibtex publications and Jupyter Notebook blog posts into your Markdown website or book. 将Bibtex转换为Markdown网站
https://docs.hugoblox.com/reference/content-types/#automatically-import-publications-from-bibtex
MIT License
356 stars 101 forks source link

Improved escaping of bibtex strings #81

Closed arichardson closed 3 years ago

arichardson commented 4 years ago

Now that the YAML is no longer written by hand but uses rueyaml we can let the library deal with escaping and pass through the raw strings. This also adds a test that an abstract with multiple paragraph now contains '\n' in the raw YAML string.

gcushen commented 4 years ago

Please can you link to the docs or code that show rueyaml has equivalent string cleaning, matching each of the current cleaning operations?

Academic's clean_bibtex_str function is widely used in the code so if it's not exactly equivalent, a number of formatting bugs could gradually appear, affecting many users.

One of the operations clean_bibtex_str performs is removing any remaining Bibtex curly braces from a string - I doubt rueyaml would perform such an operation?

arichardson commented 4 years ago

Please can you link to the docs or code that show rueyaml has equivalent string cleaning, matching each of the current cleaning operations?

Academic's clean_bibtex_str function is widely used in the code so if it's not exactly equivalent, a number of formatting bugs could gradually appear, affecting many users.

One of the operations clean_bibtex_str performs is removing any remaining Bibtex curly braces from a string - I doubt rueyaml would perform such an operation?

I've added a test to show that the curly braces are removed. Indeed, this is not handled by the rueyaml library but by the bibtex parser. Additionally, the previous code simply tried to remove all braces rather than just matched ones.

The other string cleaning operations (removing whitespace/escaping quotes) are performed by rueyaml, since this is required to ensure valid YAML output.

arichardson commented 3 years ago

ping?

arichardson commented 3 years ago

ping?

gcushen commented 3 years ago

The high priority at the moment is to close https://github.com/wowchemy/hugo-academic-cli/issues/85 , with the underlying issue negatively affecting a large number of users. Feel free to help.

We also have other, higher priority issues open awaiting contribution.

As for #81, it's very low priority and merging it has the potential to cause unwanted behaviour on a huge number of sites, similar to the merge which resulted in the issue above. Each of the cleaning rules were added for a reason, based on user feedback and I'm unconvinced that one test covers all those users' cases. The extra test case would be nice to have and perhaps can be submitted in a new PR. As for removing the old cleaning process, this requires review from the community - perhaps someone such as ionlights might be interested to review.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had any recent activity. The resources of the Academic team are limited, and so we are asking for your help. If this is a bug and you can still reproduce this error on the master branch, please reply with all of the information you have about it in order to keep the issue open. If this is a feature request, and you feel that it is still relevant and valuable, please tell us why. This issue will automatically close soon if no further activity occurs. Thank you for your contributions.