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

File paths should be platform-independent #231

Closed
rock3r opened this issue Aug 4, 2022 · 6 comments · Fixed by #240
Closed

File paths should be platform-independent #231

rock3r opened this issue Aug 4, 2022 · 6 comments · Fixed by #240
Labels
bug Something isn't working

Comments

@rock3r
Copy link
Contributor

rock3r commented Aug 4, 2022

Config file paths (and possibly other paths such as baseline) are stored in detekt.xml and read back as platform-dependent.

This means that if the detekt.xml is shared in VCS for the whole team to use, and not everyone is on the same OS (mix of POSIX and Windows users) the paths will not work for some folks and they'll be bombarded by "file not found" notifications from the Detekt plugin.

The plugin should store paths in a platform-independent way and apply the correct separators at runtime.

@arturbosch arturbosch added the bug Something isn't working label Aug 7, 2022
@arturbosch
Copy link
Member

Related #117 (comment)

Idea:
Use TableView instead of a single file chooser element in settings.
Save config paths as list.
Use a ToolbarDecorator provide add actions which trigger a file chooser dialog.

@rock3r
Copy link
Contributor Author

rock3r commented Aug 7, 2022

I'm actually working on that. Hopefully will have a PR soon. Fighting a bit with IJ :)

@arturbosch
Copy link
Member

arturbosch commented Aug 7, 2022

I'm actually working on that. Hopefully will have a PR soon. Fighting a bit with IJ :)

Ah, cool! Good that I read your comment before starting it too :).

Just to be sure (if you thought about migrating to Kotlin UI DSL): the Kotlin UI DSL will need a minimum IJ version of 2022.1 and Android Studio does not yet support it afaik.
I have a branch with it 326bc9f

I also fight a bit with IJ and try to report broken tests with the IntelliJ Gradle plugin 1.8 :)

@rock3r
Copy link
Contributor Author

rock3r commented Aug 7, 2022

I have tried maintaining the forms, but yeah, it wouldn't work. I'm migrating to the Kotlin DSL — it should mostly be ok, there's only one or two minor things in the DSL that weren't available (according to the inspections) before 22.1, but they can be emulated decently enough. So far, I am keeping 20.3 as the minimum target version, although tbh I think we can increase up to 21.2 if needed, which would still support AS stable (Chipmunk). I will only do it if needed, though.

My (very much WIP) work is here.

BTW on the topic of OS independence, I have asked and theoretically the only need we need to do is storing paths as POSIX, the IDE will take care of relativizing them (which currently only works on POSIX as we store paths on Windows as Windows paths which aren't supported properly, see #135).

@arturbosch
Copy link
Member

arturbosch commented Aug 7, 2022

I have tried maintaining the forms, but yeah, it wouldn't work. I'm migrating to the Kotlin DSL — it should mostly be ok, there's only one or two minor things in the DSL that weren't available (according to the inspections) before 22.1, but they can be emulated decently enough. So far, I am keeping 20.3 as the minimum target version, although tbh I think we can increase up to 21.2 if needed, which would still support AS stable (Chipmunk). I will only do it if needed, though.

Yes please, if you are sure about Android lets go with 21.2.
ui.dsl.builder should be version 2 of Kotlin UI DSL ? So it can't work with 20.3 right?

My (very much WIP) work is here.

BTW on the topic of OS independence, I have asked and theoretically the only need we need to do is storing paths as POSIX, the IDE will take care of relativizing them (which currently only works on POSIX as we store paths on Windows as Windows paths which aren't supported properly, see #135).

Looks good! If you open a WIP-PR I can write some comments if you like (treatAsWarnings = false).

I see you upgrade the intellij plugin. How did you solve #211 ?

@rock3r
Copy link
Contributor Author

rock3r commented Aug 7, 2022

@arturbosch I have a couple of questions for you, I pinged you on the Kotlin Slack, but I am not sure you actually use it. Let me know if you prefer other ways to get in touch.

@rock3r rock3r mentioned this issue Aug 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants