Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
fillipe-gsm opened this issue Jun 3, 2024 · 12 comments
Closed

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

fillipe-gsm opened this issue Jun 3, 2024 · 12 comments

Comments

@fillipe-gsm
Copy link

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

mcauley-penney commented Jun 3, 2024

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
Copy link
Owner

mcauley-penney commented Jun 3, 2024

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
Copy link
Author

fillipe-gsm commented Jul 12, 2024

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:

  • In the absence of .editorconfig, tidy.nvim removes whitespaces and blank lines as expected, but the indentation is wrong because of tree-sitter's defaults or vim-sleuth's wrong adjustment;
  • In the presence of .editorconfig, tidy.nvim works and the indentation is correct (my desired behavior);
  • If I add a trim_trailing_whitespace = true line to .editorconfig it also removes everything;
  • If I add a trim_trailing_whitespace = false line to .editorconfig it removes blank lines but not trailing whitespaces as expected.

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

@mcauley-penney
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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

@okuuva
Copy link

okuuva commented Sep 9, 2024

I can also confirm this works as expected. @mcauley-penney was there some blocker for this not being merged to main yet?

@mcauley-penney
Copy link
Owner

I think I just forgot tbh. I'll merge it now so I don't forget again 👍🏻 thanks for mentioning it!

@mcauley-penney
Copy link
Owner

Closed by 58fcc63

@okuuva
Copy link

okuuva commented Sep 10, 2024

I think I just forgot tbh. I'll merge it now so I don't forget again 👍🏻 thanks for mentioning it!

No, thank you for making an already awesome plugin even more awesome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants