distantnative / retour-for-kirby

Kirby CMS plugin to manage redirects and track 404s right from the Panel
https://distantnative.com/retour-for-kirby/
MIT License
135 stars 15 forks source link

4.0: Empty line may currupt retour.yml #283

Closed nilshoerrmann closed 2 years ago

nilshoerrmann commented 2 years ago

We've got a list of old URLs from a legacy CMS that we'd like to add to Retour. Because of the number of routes, creating retour.yml in the editor was easier than using the interface. The editor formats YAML using Prettier resulting in the following example file:

redirects:
  - from: wechselblick-festival-2018-ein-erlebnis-der-vielfalt-vor-unserer-haustür.html
    to: replace
    status: 301
    priority: false
    comment: ?file=files/projekte/2015/Sommerkino%202015/Braunschweiger%20Zeitung%2014.07.2015.pdf

Everything load perfectly fine in the Retour interface. But after adding a new entry via the panel, the formatting changes on save:

redirects:
  - 
    from: loesch/mich
    to: test
    status: null
    priority: false
    comment: null
  - 
    from: >
      wechselblick-festival-2018-ein-erlebnis-der-vielfalt-vor-unserer-haustür.html
    to: replace
    status: 301
    priority: false
    comment: '?file=files/projekte/2015/Sommerkino%202015/Braunschweiger%20Zeitung%2014.07.2015.pdf'

When I now adjust any redirect in my editor, Prettier formats the content again (note the empty line that's added for whatever reason):

redirects:
  - from: loesch/mich
    to: test
    status: null
    priority: false
    comment: null
  - from: >
      wechselblick-festival-2018-ein-erlebnis-der-vielfalt-vor-unserer-haustuer.html

    to: replace
    status: 301
    priority: false
    comment: '?file=files/projekte/2015/Sommerkino%202015/Braunschweiger%20Zeitung%2014.07.2015.pdf'

Saving again from the panel and now all the data is corrupted (check the gibberish in the second definition):

redirects:
  - 
    from: loeschmich
    to: test
    status: null
    priority: false
    comment: null
  - 
    from: |
      wechselblick-festival-2018-ein-erlebnis-der-vielfalt-vor-unserer-haustuer.html
      : replace atus: 301 iority: false mment: '?file=files/projekte/2015/Sommerkino%202015/Braunschweiger%20Zeitung%2014.07.2015.pdf'
    to: null
    status: null
    priority: false
    comment: null
nilshoerrmann commented 2 years ago

I've seen there this note in the readme:

  // Absolut path for location of redirects config
  // Default: site/config/retour.yml
  // (allows for yml and json files/formats)

It might be a good idea to disallow YAML completely for the redirects and always store in JSON format to prevent this issue.

nilshoerrmann commented 2 years ago

As the errors occured on save and reading seems to be fine: maybe Retour could automatically convert retour.yml to retour.json if the later does not exist yet.

nilshoerrmann commented 2 years ago

I've tried validating the YAML formatted by Prettier that Retour was not able to process correctly and it seems to be valid (the one with the empty line). So this might be a problem of the Kirby core as well when writing YAML.

distantnative commented 2 years ago

Since I am simply using Kirby's Data class to read and write the yaml file, I think this might be an issue of the Spyc library that Kirby is using for YAML. But nothing that I would be able to fix for this plugin then.

It might be a good idea to disallow YAML completely for the redirects and always store in JSON format to prevent this issue.

Won't do this since people have existing YAML setups and some prefer it due to easier readability.

I am also not a big fan of reading YAML and then magically saving as JSON without the user knowing. I think a manual conversion from YAML to JSON should be the preferred way for users that are facing issues with a config file in YAML format.