-
Notifications
You must be signed in to change notification settings - Fork 503
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
[Question] What is the reason to drop support for absolute paths in file patterns? #1131
Comments
My understanding is that some complex platforms that unify different static analyzers can be affected by this change. |
These rules are the first step in providing a lightweight way of ensuring projects use consistent formatting.
As mentioned in the issue description, the README and the attached PR#999 is really vague on the details. I might have missed some of the documentation around it, but I also don't see a way to match against a path using the I would also like to know if there are any plans to bring back this functionality. I'm not sure how popular the negated blobs feature was, but it seems like a really expensive bargain if we can no longer specify full path files. This makes it extremely hard to use Would you please reconsider adding back support for full path matches? Maybe even behind a CLI flag if needed...? |
fails as well. No globs involved. |
This change is needed because KTLint, starting from version 0.41.0, dropped support for absolute paths (pinterest/ktlint#1131) While running pre-commit hooks the injected paths are paths relative to GIT_ROOT, so the presence of absolute paths is merely an artifact of testing.
Hi folks, we'll look into this, as it sounds like it's blocking a number of people from upgrading. |
Thanks @shashachu! this will be brilliant :) |
Is it expected to be fixed in new reales? |
@petertrr 's PR to add back support for absolute paths has been landed to master. Once the SNAPSHOT builds, it'd be good for folks in this thread to test it out to ensure it works. |
@shashachu is there an ETA for 0.43.0 (or maybe 0.42.2) release? We have tested the snapshot on our side, it works as expected. |
0.43.0 will be integrated into diktat, so there will be a double check of the stability of this version |
Affects us as well. Very tricky this days to pin ktlint to |
@kirillgroshkov Petya has fixed it in #1199 . |
The latest ktlint update removed the support for absolute paths as arguments. pinterest/ktlint#1131 Github workflow used to prepend the checkout source to make paths absolute and now it does not work anymore. This PR replaces it with `..` since we run ktlint inside `activity` project. There is probably a better shell magic to get this but I think .. is good enough for now. (there does not seem to be a consistent solution between mac and linux for relative paths). Bug: n/a Test: locally verified that relative paths with .. works by manually breaking a file's style in room and invoked ktlint in activity as the workflow does.
The latest ktlint update removed the support for absolute paths as arguments. pinterest/ktlint#1131 Github workflow used to prepend the checkout source to make paths absolute and now it does not work anymore. This PR replaces it with `..` since we run ktlint inside `activity` project. There is probably a better shell magic to get this but I think .. is good enough for now. (there does not seem to be a consistent solution between mac and linux for relative paths). Bug: n/a Test: locally verified that relative paths with .. works by manually breaking a file's style in room and invoked ktlint in activity as the workflow does. This is an imported pull request from #247. Resolves #247 Github-Pr-Head-Sha: da89915 GitOrigin-RevId: 80adfcd Change-Id: Ie6ec09c66a57f89970aff9da581135769b43ca9f
Because the fixed version is still not released and our users were reporting this issue over and over, I've wrote down an instruction on how to install ("pin") the working Leaving it here in case it's useful for someone else. Install specific version of ktlintCurrently, the latest version of ktlint is broken (doesn't support absolute urls, while we need that support). You'll need First, uninstall whatever version you have, with Please go to the github release page for that version: https://github.com/pinterest/ktlint/releases/tag/0.40.0 Download the Then, move this mv ~/Downloads/ktlint /usr/local/bin
chmod +x /usr/local/bin/ktlint Test that it works: |
I'm seeing the new exception at
ktlint/ktlint/src/main/kotlin/com/pinterest/ktlint/internal/FileUtils.kt
Line 89 in 7e2c7fd
PathMatcher
, but I don't quite understand the reason for this particular restriction.Is it a permanent decision or can this behavior be altered in future releases? Or does it break something in expected workflow?
The text was updated successfully, but these errors were encountered: