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

Paths with native Windows separator \ don't seem to be matched properly. #16

Closed
ams-tschoening opened this issue Oct 26, 2018 · 8 comments
Labels

Comments

@ams-tschoening
Copy link

ams-tschoening commented Oct 26, 2018

I'm working on Windows and tried to use gradle-eclipseconfig to manage charset settings in Eclipse project managed by Gradle. The examples provided by the project didn't work and I think I finally found the problem in the fact that the plugin forwards native paths to EditorConfig. While EditorConfig seems to have some recognition of \ in section-globs, it doesn't seem to normalize paths to match in the end. So the simple section [*] doesn't match Windows paths.

Have a look at the following debugging output:

11:01:56.533 [ERROR] [org.gradle.api.Project] C:\[...]\Dummy.java -> C:/[...]/Dummy.java
11:01:56.535 [ERROR] [system.err] .*/[^/]*
11:01:56.538 [ERROR] [system.err] .*/[^/]*
11:01:56.538 [ERROR] [org.gradle.api.Project] [:] -> [charset:utf-8, trim_trailing_whitespace:true, indent_style:space, indent_size:2, tab_width:2]
11:01:56.543 [ERROR] [system.err] .*/[^/]*

The outputs on STDERR are from EditorConfig by setting the following on the shell:

set JAVA_OPTS=-Deditorconfig.debug=true

These are the regular expressions matched against the paths and as you can see, those are using / instead of \ always with my very simple section. In my opinion, before matching the file paths those should be normalized to correspond with whatever is used in the patterns as separator.

https://github.com/editorconfig/editorconfig-core-java/blob/master/src/main/java/org/editorconfig/core/EditorConfig.java#L242

@ppalaga
Copy link
Contributor

ppalaga commented Oct 26, 2018

gradle-eclipseconfig uses this fork of editorconfig-core-java https://github.com/stempler/editorconfig-core-java/tree/maven-central-standardout probably because @denofevil is not interested in releasing editorconfig-core-java to Maven Central, see #6. So you may be luckier to report this issue on the fork.

@xuhdev
Copy link
Member

xuhdev commented Oct 26, 2018

Can you give an example input and expected output? I suspect that this is intended, as EditorConfig files are supposed to be used cross-platform: We intentionally do not recognize backslashes as path separators, as backslashes can be part of a file name on other systems.

@ams-tschoening
Copy link
Author

The examples are in my first post already: An arbitrary native Windows-path as source using \ and a config file containing one section [*] only. That doesn't match the path because it is transformed to the regular expression .*/[^/]*. That expression seems to be expected to match a path with / only.

OTOH, the docs say the following:

 * | Matches any string of characters, except path separators (/)
** | Matches any string of characters

So am I expected to use one section [**] only instead? But your reg exp clearly matches path separators in contrast what the docs are saying.

The project I'm using, gradle-eclipseconfig, suggests to use [*] and did use absolute paths always and that did work on Linux. Which make sense looking at your reg exp, it only can't work for native Windows paths this way.

Only forward slashes (/, not backslashes) are used as path separators[...]

This reads like one needs to normalize paths to / always, either EditorConfig or all callers individually. The latter doesn't make much sense to me. Else I don't see how sections with / can match native Windows paths ever.

@xuhdev
Copy link
Member

xuhdev commented Oct 26, 2018

@ams-tschoening Are you a plugin developer using editorconfig-core-java or a user using some editor plugin?

@xuhdev
Copy link
Member

xuhdev commented Oct 26, 2018

I'm just so confused by everything you said. Are you using the Eclipse plugin? If so, the plugin should normalize the path before it passes the path to editorconfig-core-java, and hence should be a bug reported there. If you are a plugin developer and would like to use this core library, then you should normalize the path before you pass it to editorconfig-core-java.

@ppalaga
Copy link
Contributor

ppalaga commented Oct 26, 2018

He is a user of gradle-eclipseconfig https://github.com/stempler/gradle-eclipseconfig that uses a fork of editorconfig-core-java

@ams-tschoening
Copy link
Author

If so, the plugin should normalize the path before it passes the path to editorconfig-core-java[...]

Wouldn't it be easier for your users if you normalize the given path to / always internally? That would have prevented the problem I was running into entirely. If you think otherwise, feel free to close this issue.

@xuhdev
Copy link
Member

xuhdev commented Oct 27, 2018

@ams-tschoening As a user, you don't input anything into the core library directly, but input into gradle-eclipseconfig, which in turn passes the path to the core library. In the design of the architecture of EditorConfig, gradle-eclipseconfig should handle the normalization process, and therefore this bug should be reported in gradle-eclipseconfig's issue tracker, which is not governed by us.

In other words, I think your concern regarding the expected output makes sense, but I think this is a bug belong to gradle-eclipseconfig.

@xuhdev xuhdev added the invalid label Oct 27, 2018
@xuhdev xuhdev closed this as completed Oct 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants