Swap to from js-yaml to yaml for Parsing YAML
#1227
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pjkaufman
added
bug
|
If I am understanding correctly, the regex still works when FFF:
- becomes FFF:
- So I think it should be fine to proceed with this change. I may want to add some more changes first to see how things go. |
|
I checked again to make sure and it seems it converts the value from FFF:
- to FFF:
- to FFF:
- which the regex does not support. So it is not actually feasible to swap over until all of the regex calls are fixed. However this also brings in some doubt that the underlying parser also knows that the ending version is the original version of the content. |
|
Looks like that was a bug that was present prior. It has been fixed after I reported the bug to the library. |
|
I would like to swap over to using the CST for more granularity. One place it is currently used is https://github.com/apollographql/argocd-config-updater/blob/0217630ad16edf17d8ff98239cca3c4b29426ec9/src/yaml.ts |
johannrichard
added a commit
to johannrichard/readwise-mirror
that referenced
this pull request
Jan 21, 2025
implement a fix to avoid line-breaks in long titles until platers/obsidian-linter#1227 is implemented in `platers/obsidian-linter`.
johannrichard
added a commit
to johannrichard/readwise-mirror
that referenced
this pull request
Jan 21, 2025
…and protection, and slugification of filenames, as well as a refactor of the codebase (#21) * feat: ✨ add options to "slugify" the filenames implements jsonMartin#27 will work best together with the changes proposed in https://github.com/johannrichard/readwise-mirror/tree/deduplicate-files * docs: 📝 add section on slugifying filenames in README * refactor: 🎨 reduce chatter by using `console.debug()` more often * build: 👷 move dev-dependency * feat: ✨ implement deduplication and frontmatter validation - implement deduplication based on `readwise_url` property from exports: user can select to eitehr delete or tag duplicate entries - contents of the duplicate file will be overwritten - exact filename match will be kept if existing, - otherwise, the first match will be renamed in the vault so that links can be updated - validate frontmatter yaml and display error messages or rendered frontmatter * docs: ✨ introduce frontmatter validation and file deduplication * chore: 💩 add todo to check whether the dataview plugin is the only viable way to find dupes * ci: 💚 fix `package.json` version * build: 💚 fix`semantic-release` build * build: 💚 fix semantic-release on beta branch * build: 🚀 deploy on `beta` branch automatically * refactor: ➕ move to `yaml` from `js-yaml` - change `yaml` library * fix: ✅ improve robustness for data edge cases - add line-break in sample data - use `filenamify` to create valid filenames (incl. from titles with linebreaks) if `slugify` is disabled * fix: 🐛 fix frontmatter validation (ui) - run`YAML.parse()` when validating frontmatter template * fix: 🐛 increase filename max length to 255 characters * feat: ✨ update and protect frontmatter with a new setting, it is now possible to keep existing, additional frontmatter in files, and to protect from specific fields to be updated even if they exists in the frontmatter template. This works best if you also enable deduplication. * fix: 🐛 catch additional cases - update frontmatter when deduplication is turned off - catch case where deduplication is turned off and files are not written * docs: 📝 add feature documentatio for frontmatter update and protection add documentation for frontmatter updates and protection in the `README.md` * docs: 🐛 fix documentation mistakes * feat: 🚸 only protect fields that exist in the current frontmatter - protected fields which are not present in the current file will be added (if set in the template during first sync), and - will be protected in future syncs once they exist in the current file * docs: 🚸 explain frontmatter protection in more detail * refactor: 🚸 improve display of template blocks in settings improve the display long templates in the settings screen by adjusting the textarea dynamically. * refactor: 🚸 dynamically toggle slugify option visibility only show slugify options if the option to slugify is enabled * refactor: ♻️ refactor files and separate settings, definitions, and main plugin * refactor: ♻️ refactor codebase into separate `src` directory * refactor: ♻️ further refactoring of the code base separate models from ui, services, and main * chore: 🚚 move, refactor, and clean-up code * perf: ⚡️ remove dataviee dependency and improve deduplication performance - by implementing deduplication via metadatacache, we remove a dependency to another plugin *and* improve deduplication performance at the same time (approx. by a factor of 10 for 10 duplicates of one file). * fix: 🐛 correctly update frontmatter when deduplicating when multiple files exist, make sure we properly update the frontmatter for all duplicate files if setting is enabled * style: 🎨 format code * build: 🐛 fix local deploy system after refactor * fix: 🚑️ account for filename normalization ensure we get files with a compatible composed normalized path * fix: 👽 implement fix for dealing with `js-yaml` parsing implement a fix to avoid line-breaks in long titles until platers/obsidian-linter#1227 is implemented in `platers/obsidian-linter`. * refactor: use Obsidian's buil-in `normalizePath()` function when working with filenames (e.g. to find duplicates with `getAbstractFilePath()`) * feat: ✨ deduplicate Readwise duplicates implement code to treat the case of multiple Readwise items with the same title (i.e. the same filename in Obsidian) * docs: 📝 improve the deduplication documentation explain the use-case for deduplication better and explain *local* and *remote* duplicates in the documentation * style: 🎨 improve some of the new code and remove unused comments
johannrichard
added a commit
to johannrichard/readwise-mirror
that referenced
this pull request
Jan 24, 2025
* feat: ✨ add options to "slugify" the filenames implements jsonMartin#27 will work best together with the changes proposed in https://github.com/johannrichard/readwise-mirror/tree/deduplicate-files * docs: 📝 add section on slugifying filenames in README * refactor: 🎨 reduce chatter by using `console.debug()` more often * build: 👷 move dev-dependency * feat: ✨ implement deduplication and frontmatter validation - implement deduplication based on `readwise_url` property from exports: user can select to eitehr delete or tag duplicate entries - contents of the duplicate file will be overwritten - exact filename match will be kept if existing, - otherwise, the first match will be renamed in the vault so that links can be updated - validate frontmatter yaml and display error messages or rendered frontmatter * docs: ✨ introduce frontmatter validation and file deduplication * chore: 💩 add todo to check whether the dataview plugin is the only viable way to find dupes * ci: 💚 fix `package.json` version * build: 💚 fix`semantic-release` build * build: 💚 fix semantic-release on beta branch * build: 🚀 deploy on `beta` branch automatically * refactor: ➕ move to `yaml` from `js-yaml` - change `yaml` library * fix: ✅ improve robustness for data edge cases - add line-break in sample data - use `filenamify` to create valid filenames (incl. from titles with linebreaks) if `slugify` is disabled * fix: 🐛 fix frontmatter validation (ui) - run`YAML.parse()` when validating frontmatter template * fix: 🐛 increase filename max length to 255 characters * feat: ✨ update and protect frontmatter with a new setting, it is now possible to keep existing, additional frontmatter in files, and to protect from specific fields to be updated even if they exists in the frontmatter template. This works best if you also enable deduplication. * fix: 🐛 catch additional cases - update frontmatter when deduplication is turned off - catch case where deduplication is turned off and files are not written * docs: 📝 add feature documentatio for frontmatter update and protection add documentation for frontmatter updates and protection in the `README.md` * docs: 🐛 fix documentation mistakes * feat: 🚸 only protect fields that exist in the current frontmatter - protected fields which are not present in the current file will be added (if set in the template during first sync), and - will be protected in future syncs once they exist in the current file * docs: 🚸 explain frontmatter protection in more detail * refactor: 🚸 improve display of template blocks in settings improve the display long templates in the settings screen by adjusting the textarea dynamically. * refactor: 🚸 dynamically toggle slugify option visibility only show slugify options if the option to slugify is enabled * refactor: ♻️ refactor files and separate settings, definitions, and main plugin * refactor: ♻️ refactor codebase into separate `src` directory * refactor: ♻️ further refactoring of the code base separate models from ui, services, and main * chore: 🚚 move, refactor, and clean-up code * perf: ⚡️ remove dataviee dependency and improve deduplication performance - by implementing deduplication via metadatacache, we remove a dependency to another plugin *and* improve deduplication performance at the same time (approx. by a factor of 10 for 10 duplicates of one file). * fix: 🐛 correctly update frontmatter when deduplicating when multiple files exist, make sure we properly update the frontmatter for all duplicate files if setting is enabled * style: 🎨 format code * build: 🐛 fix local deploy system after refactor * fix: 🚑️ account for filename normalization ensure we get files with a compatible composed normalized path * fix: 👽 implement fix for dealing with `js-yaml` parsing implement a fix to avoid line-breaks in long titles until platers/obsidian-linter#1227 is implemented in `platers/obsidian-linter`. * refactor: use Obsidian's buil-in `normalizePath()` function when working with filenames (e.g. to find duplicates with `getAbstractFilePath()`) * feat: ✨ deduplicate Readwise duplicates implement code to treat the case of multiple Readwise items with the same title (i.e. the same filename in Obsidian) * docs: 📝 improve the deduplication documentation explain the use-case for deduplication better and explain *local* and *remote* duplicates in the documentation * style: 🎨 improve some of the new code and remove unused comments * refactor: 🔀 merging some upstream changes * build: 🐛 fix release packaging add missing `src/ui/styles/styles.css` from src refactoring to release * feat: ✨ implement more robust error-handling for Readwise API catch error 429 (Rate Limit) in case header can't be read, catch (rare) empty results, handle other HTTP errors than 429 (@coderrabbit suggestions)
johannrichard
added a commit
to johannrichard/readwise-mirror
that referenced
this pull request
Jan 24, 2025
* feat: ✨ add options to "slugify" the filenames implements jsonMartin#27 will work best together with the changes proposed in https://github.com/johannrichard/readwise-mirror/tree/deduplicate-files * docs: 📝 add section on slugifying filenames in README * refactor: 🎨 reduce chatter by using `console.debug()` more often * build: 👷 move dev-dependency * feat: ✨ implement deduplication and frontmatter validation - implement deduplication based on `readwise_url` property from exports: user can select to eitehr delete or tag duplicate entries - contents of the duplicate file will be overwritten - exact filename match will be kept if existing, - otherwise, the first match will be renamed in the vault so that links can be updated - validate frontmatter yaml and display error messages or rendered frontmatter * docs: ✨ introduce frontmatter validation and file deduplication * chore: 💩 add todo to check whether the dataview plugin is the only viable way to find dupes * ci: 💚 fix `package.json` version * build: 💚 fix`semantic-release` build * build: 💚 fix semantic-release on beta branch * build: 🚀 deploy on `beta` branch automatically * refactor: ➕ move to `yaml` from `js-yaml` - change `yaml` library * fix: ✅ improve robustness for data edge cases - add line-break in sample data - use `filenamify` to create valid filenames (incl. from titles with linebreaks) if `slugify` is disabled * fix: 🐛 fix frontmatter validation (ui) - run`YAML.parse()` when validating frontmatter template * fix: 🐛 increase filename max length to 255 characters * feat: ✨ update and protect frontmatter with a new setting, it is now possible to keep existing, additional frontmatter in files, and to protect from specific fields to be updated even if they exists in the frontmatter template. This works best if you also enable deduplication. * fix: 🐛 catch additional cases - update frontmatter when deduplication is turned off - catch case where deduplication is turned off and files are not written * docs: 📝 add feature documentatio for frontmatter update and protection add documentation for frontmatter updates and protection in the `README.md` * docs: 🐛 fix documentation mistakes * feat: 🚸 only protect fields that exist in the current frontmatter - protected fields which are not present in the current file will be added (if set in the template during first sync), and - will be protected in future syncs once they exist in the current file * docs: 🚸 explain frontmatter protection in more detail * refactor: 🚸 improve display of template blocks in settings improve the display long templates in the settings screen by adjusting the textarea dynamically. * refactor: 🚸 dynamically toggle slugify option visibility only show slugify options if the option to slugify is enabled * refactor: ♻️ refactor files and separate settings, definitions, and main plugin * refactor: ♻️ refactor codebase into separate `src` directory * refactor: ♻️ further refactoring of the code base separate models from ui, services, and main * chore: 🚚 move, refactor, and clean-up code * perf: ⚡️ remove dataviee dependency and improve deduplication performance - by implementing deduplication via metadatacache, we remove a dependency to another plugin *and* improve deduplication performance at the same time (approx. by a factor of 10 for 10 duplicates of one file). * fix: 🐛 correctly update frontmatter when deduplicating when multiple files exist, make sure we properly update the frontmatter for all duplicate files if setting is enabled * style: 🎨 format code * build: 🐛 fix local deploy system after refactor * fix: 🚑️ account for filename normalization ensure we get files with a compatible composed normalized path * fix: 👽 implement fix for dealing with `js-yaml` parsing implement a fix to avoid line-breaks in long titles until platers/obsidian-linter#1227 is implemented in `platers/obsidian-linter`. * refactor: use Obsidian's buil-in `normalizePath()` function when working with filenames (e.g. to find duplicates with `getAbstractFilePath()`) * feat: ✨ deduplicate Readwise duplicates implement code to treat the case of multiple Readwise items with the same title (i.e. the same filename in Obsidian) * docs: 📝 improve the deduplication documentation explain the use-case for deduplication better and explain *local* and *remote* duplicates in the documentation * style: 🎨 improve some of the new code and remove unused comments * refactor: 🔀 merging some upstream changes * build: 🐛 fix release packaging add missing `src/ui/styles/styles.css` from src refactoring to release * feat: ✨ implement more robust error-handling for Readwise API catch error 429 (Rate Limit) in case header can't be read, catch (rare) empty results, handle other HTTP errors than 429 (@coderrabbit suggestions) * fix: 🐛 fix wrong variable names correctly catch missing `created_at` and `updated_at` (was `highlighted_at`) * fix: 📌 pin `semantic-release-obsidian-plugin` pin osbidian release plugin in build scripts and use npm version instead of github version
1 task
… converting back to a string
|
I believe the CST is now being properly used to help retain the YAML as closely to what it was originally. This does not currently do anything for the regex used to grab values and move them anywhere except in the yaml-key-sort logic right now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1082 #912 #992 #660 #632 #649 #1258
The following are a list of tickets that are potentially fixable with the merger of this change, but logic will need to change in order to properly handle these scenarios:
Relates to #715 #1217 #617
The main reason for making this change is getting closer to having a round trip parser for the Linter to use on the YAML. This change helps with supporting more YAML syntax as well as having a less buggy experience for end users. It should result in fewer instances where things get broken.
It should do its best to keep things as they were, but there is no guarantee that that will be the case for all formats. However it will do its best given that it is using the CST instead of the AST for serializing to YAML.