zkry / yaml-pro

Edit YAML in Emacs like a pro
GNU General Public License v3.0
137 stars 9 forks source link

yaml-pro-indent-subtree results in bad parse #25

Open antifuchs opened 1 year ago

antifuchs commented 1 year ago

Just playing around with yaml-pro and it's really really cool! However, I found an interaction (in yaml-ts-mode) where indenting a subtree too far will result in a yaml file that no longer parses, which prevents you from unindenting the subtree (undo works, but it would be nice if unindent always could reduce indentation?):

    with:
      cargo_test_args: $${{matrix.cargo_test_args}}
      manifest_dir: ${manifest_dir}
      rust_toolchain: $${{matrix.rust_toolchain}}
      # ^ point here on the line above
      apt_install_packages: ${jsonencode(join(" ", apt_install_packages))}

Activating yaml-pro-indent-subtree at the s in "rust_toolchain" indents that key and comment below it:

    with:
      cargo_test_args: $${{matrix.cargo_test_args}}
      manifest_dir: ${manifest_dir}
        rust_toolchain: $${{matrix.rust_toolchain}}
        # ^ point here      
      apt_install_packages: ${jsonencode(join(" ", apt_install_packages))}

But then you can no longer unindent the subtree: Invoking yaml-pro-unindent-subtree results in: yaml-parse-tree: Unable to parse YAML. Parser finished before end of input 600/792 [2 times] - correctly! The yaml code in the buffer no longer parses!

Would it make sense for the indent/unindent operations to attempt to parse the resulting parent tree & refuse to indent if the result would fail to parse? The unindent operation does just that when unindenting at column 0, so it would make sense to catch more error cases, I think...

zkry commented 1 year ago

Thanks for posting this! The suggestion of ensuring that the resulting indentation will parse should be pretty easy to implement with treesitter. I'll have a go at implementing this.

zkry commented 6 months ago

So I got the treesitter indent/unintend functions to check the tree after indentation to see if it's a vailid tree. I also allowed the option to override this with the C-u prefix.