mcauley-penney / tidy.nvim

A small Neovim plugin to remove trailing whitespace and empty lines at end of file on every save
107 stars 13 forks source link

Not working in the presence of a `.editorconfig` file #17

Open fillipe-gsm opened 1 month ago

fillipe-gsm commented 1 month ago

Hi, I use vim-sleuth to automatically define the file indentation. Since it was erroneously setting indentation for R files, I included a EditorConfig file in my root directory. While this fixed the indentation, I noticed the whitespaces and blank lines were not being removed anymore.

At first I thought this was something with the R language, but it happens consistently with files of any extension I put in this directory. And, if I remove the .editorconfig file, tidy starts working again.

The smallest project to replicate the issue is with an empty folder with a .editorconfig file with the contents:

root = true

and any files with any extension in this folder or any subfolder stop getting tidied on save.

I checked the plugin is still loaded with Lazy, and even by running lua require("tidy").run() manually nothing happens when the editorconfig file is there.

Is this something expected? I noticed this line in the code apparently disabling it in the presence of this file, but I am not that fluent in Lua to ensure my analysis is correct.

mcauley-penney commented 1 month ago

Thank you for opening an issue! I don't think I've ever used an editorconfig file for anything other than implementing functionality for this plugin, so this is an area that I like to have issues for.

Yes, if an editorconfig file is present, tidy is disabled (see this PR). Given that I don't have cause to use this feature often, I deferred to the judgement of users of the plugin for how it should behave when an editorconfig file is present. iiuc, the functionality that tidy provides can also be had through editorconfig functionality.

What do you think about all of this?

fillipe-gsm commented 1 month ago

To be honest I just heard about the EditorConfig and it was very convenient to fix the tab sizes that vim-sleuth was getting wrong, although it seems to be able to handle much more than that.

Thus, while I think it was a safe move to disable the plugin in its presence, we may get into this corner case where, e.g., I want to use editorconfig to set tab sizes but still want tidy to clean my files.

Since you asked, I think it would be nice to use editorconfig to decide how tidy would work. This would be nice given that it is something neovim already supports and could act as some kind of plugin configuration, but I don't know how hard it would be to do and maintain.

(If going with this route, apparently the trim_trailing_whitespace and insert_final_newline are the most important configurations to support.)

I would be happy to serve as a guinea pig to test some it if you decide to implement it.

Alternatively, I am also okay with an option to completely ignore the editorconfig and keep working as usual. We could see conflicts in the previous two configurations but as long as this is documented and disabled by default it should be fine.

mcauley-penney commented 1 month ago

Since you asked, I think it would be nice to use editorconfig to decide how tidy would work. This would be nice given that it is something neovim already supports and could act as some kind of plugin configuration, but I don't know how hard it would be to do and maintain.

To clarify the requirements, you'd like something where if .editorconfig is found, tidy should check what functionality it defines and provide what isn't set?

Example 1

You have an .editorconfig file and it contains

[*]
root = true

So, tidy sees this and detects that trimming whitespace and leaving the last newline are not defined. In that case, you'd like for tidy to continue with all of its normal behavior

Example 2

Given the below .editorconfig file

[*]
trim_trailing_whitespace = false
insert_final_newline = true

You'd like tidy to detect these settings and remove newlines except for the last one and not trim whitespace?

Is this accurate? All of this can be done pretty easily.

mcauley-penney commented 1 month ago

An update since my last comment: I've implemented the changes and pushed them to a branch called provide-undefined-editorconfig-behavior. If you load this branch in your plugin manager, you can set provide_undefined_editorconfig_behavior = true and tidy will work on buffers even when an editorconfig file is found. Its behavior:

1) Because trimming newlines doesn't seem to interfere with editorconfig's insert_final_newline option, it just always trims newlines regardless of what is set or not set for this option.

2) tidy will trim whitespace as long as editorconfig's trim_trailing_whitespace is undefined. If it is defined, regardless of whether it is true or false, tidy will continue to remove newlines but will not trim whitespace. It makes sense: if trim_trailing_whitespace is false, the user doesn't want whitespace trimmed. If true, editorconfig will do it for you.

All of this behavior is opt-in, so it is off by default. Tidy will continue to default to just not running at all if an .editorconfig file is found.

If this fits your needs and it works well, I will merge this into main.

fillipe-gsm commented 1 week ago

Hey, I am sorry for the late reply, but I managed to test the plugin on provide-undefined-editorconfig-behavior.

I tested on a R language file because it is one example when my tree-sitter does not respect the recommended indentation (it adds 8 spaces on indent instead of 2).

For reference, my nvim pertaining configuration is:

{
  'mcauley-penney/tidy.nvim',
  config = true,
  branch = 'provide-undefined-editorconfig-behavior',
  opts = {
    provide_undefined_editorconfig_behavior = true,
  },
}

(I use Lazy to manage plugins)

and I created the following .editorconfig file in the root of a folder with R files:

[*.R]
indent_style = space
indent_size = 2

I noticed that:

So thanks a lot for your work and response! In my view this is good to be merged.

mcauley-penney commented 1 week ago

I tested on a R language file because it is one example when my tree-sitter does not respect the recommended indentation (it adds 8 spaces on indent instead of 2).

In the absence of .editorconfig, tidy.nvim removes whitespaces and blank lines as expected, but the indentation is wrong;

To be explicit, does this appear to be coming from Tidy or is this the same behavior that you get with treesitter even without Tidy? If this is the plugin's fault, I'll happily look into before merging.

fillipe-gsm commented 1 week ago

I tested on a R language file because it is one example when my tree-sitter does not respect the recommended indentation (it adds 8 spaces on indent instead of 2).

In the absence of .editorconfig, tidy.nvim removes whitespaces and blank lines as expected, but the indentation is wrong;

To be explicit, does this appear to be coming from Tidy or is this the same behavior that you get with treesitter even without Tidy? If this is the plugin's fault, I'll happily look into before merging.

Oh no, sorry, I didn't express myself correctly.

The wrong indentation is not related to tidy.nvim; it is either tree-sitter's defaults or vim-sleuth's wrong adjustment. I updated my comment to reflect that.

The initial issue was tidy.nvim being disabled in the presence of .editorconfig, and this looked fixed in your new branch according to my experiments , so thanks again for that.

mcauley-penney commented 1 week ago

For sure, no worries! Thank you for clarifying 💯 In that case, I will look at merging this later today and I'm glad to hear that its working for your use case