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

Wrong file path resolved when using --editorconfig #1551

Closed
StenSoft opened this issue Jul 19, 2022 · 3 comments · Fixed by #1580
Closed

Wrong file path resolved when using --editorconfig #1551

StenSoft opened this issue Jul 19, 2022 · 3 comments · Fixed by #1580
Labels
Milestone

Comments

@StenSoft
Copy link

When using --editorconfig option, a wrong path will be resolved.

Without --editorconfig:

12:13:49.290 [pool-1-thread-1] TRACE com.pinterest.ktlint.core.internal.EditorConfigLoader - Resolving .editorconfig files for app/src/main/kotlin/com/example/Example1.kt file path:
12:13:49.290 [pool-1-thread-2] TRACE com.pinterest.ktlint.core.internal.EditorConfigLoader - Resolving .editorconfig files for app/src/main/kotlin/com/example/Example2.kt file path:

With --editorconfig=/tmp/editorconfig:

12:15:04.346 [pool-1-thread-2] TRACE com.pinterest.ktlint.core.internal.EditorConfigLoader - Resolving .editorconfig files for /tmp/editorconfig/Example1.kt file path:                                                                                                                                                                                                                                                   
12:15:04.348 [pool-1-thread-1] TRACE com.pinterest.ktlint.core.internal.EditorConfigLoader - Resolving .editorconfig files for /tmp/editorconfig/Example2.kt file path:

Notice that the file's director name has been replaced with the editorconfig path. Path-specific rules for e.g. [**/example/*.kt] won't be applied.

@paul-dingemans
Copy link
Collaborator

With --editorconfig=/tmp/editorconfig:

12:15:04.346 [pool-1-thread-2] TRACE com.pinterest.ktlint.core.internal.EditorConfigLoader - Resolving .editorconfig files for /tmp/editorconfig/Example1.kt file path:                                                                                                                                                                                                                                                   
12:15:04.348 [pool-1-thread-1] TRACE com.pinterest.ktlint.core.internal.EditorConfigLoader - Resolving .editorconfig files for /tmp/editorconfig/Example2.kt file path:

The logging above is indeed inccorrect.

Path-specific rules for e.g. [**/example/*.kt] won't be applied.

Whenever the --editorconfig option is used, all .editorconfig files on the file path are ignored. I expect that this was a consicious design choice but it has not been documented (or at least I can not find it). Suppose that an .editorconfig file is found on the file path. Is the setting from the specified .editorconfig file more or less import than from the file found on the file path? I expect that this differs per user.

But the least that could be done, is to mention the current behavior in the CLI help.

@paul-dingemans paul-dingemans added this to the 0.47.0 milestone Jul 21, 2022
@StenSoft
Copy link
Author

Whenever the --editorconfig option is used, all .editorconfig files on the file path are ignored. I expect that this was a consicious design choice but it has not been documented (or at least I can not find it). Suppose that an .editorconfig file is found on the file path. Is the setting from the specified .editorconfig file more or less import than from the file found on the file path? I expect that this differs per user.

That is fine, I expected that.

The problem is with path-based rules in the provided EditorConfig. If the EditorConfig looks like this:

[*.{kt,kts}]
max_line_length = 140

[app/**/example/*.kt]
disabled_rules = filename

Then when it is found by directory traversal, both max_line_length and disabled_rules rules are applied to app/src/main/kotlin/com/example/Example1.kt, but when it is specified via the --editorconfig option, only the max_line_length rule is applied because the rewritten file name does not match the latter section.

@paul-dingemans
Copy link
Collaborator

Tnx for this clarification. That indeed sounds like a bug.

paul-dingemans added a commit to paul-dingemans/ktlint that referenced this issue Aug 13, 2022
Deprecate ExperimentalParams.editorConfigDefaults in favor of new parameter
ExperimentalParams.editorConfigDefaults. When used in the old implementation
this resulted in ignoring all ".editorconfig" files on the path to the file.
The new implementation uses properties from the "editorConfigDefaults"
parameter only when no ".editorconfig" files on the path to the file supplies
this property for the filepath.
Closes pinterest#1551

API consumers can easily create the EditConfigDefaults by calling
 "EditConfigDefaults.load(path)" or creating it programmatically.

The CLI still supports the "--editorconfig=" option but has improved support.
The path given can be either be a path to file or directory. In case of a
directory path, it is expected that the directory does contain a file with
name ".editorconfig". In of a file path, any valid file name is accepted. The
path can be relative or absolute. Depending on the OS, the "~" at the start of
the path is accepted as well.

BaseCLITest no longer always waits 3 seconds for completion of the asynchronous
process. Once the process is started, it checks every 100 ms whether the process
is still alive (e.g. is running) and stops polling otherwise resulting in better
performance (most notable on local machine). The maximum duration of the CLI
test has been increased to 10 seconds.
paul-dingemans added a commit that referenced this issue Aug 18, 2022
* Improve support of default editorconfig properties

Deprecate ExperimentalParams.editorConfigDefaults in favor of new parameter
ExperimentalParams.editorConfigDefaults. When used in the old implementation
this resulted in ignoring all ".editorconfig" files on the path to the file.
The new implementation uses properties from the "editorConfigDefaults"
parameter only when no ".editorconfig" files on the path to the file supplies
this property for the filepath.
Closes #1551

API consumers can easily create the EditConfigDefaults by calling
 "EditConfigDefaults.load(path)" or creating it programmatically.

The CLI still supports the "--editorconfig=" option but has improved support.
The path given can be either be a path to file or directory. In case of a
directory path, it is expected that the directory does contain a file with
name ".editorconfig". In of a file path, any valid file name is accepted. The
path can be relative or absolute. Depending on the OS, the "~" at the start of
the path is accepted as well.

BaseCLITest no longer always waits 3 seconds for completion of the asynchronous
process. Once the process is started, it checks every 100 ms whether the process
is still alive (e.g. is running) and stops polling otherwise resulting in better
performance (most notable on local machine). The maximum duration of the CLI
test has been increased to 10 seconds.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants