Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #100 +/- ##
==========================================
+ Coverage 79.26% 79.28% +0.02%
==========================================
Files 12 12
Lines 786 787 +1
==========================================
+ Hits 623 624 +1
Misses 163 163
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Your PR no longer requires formatting changes. Thank you for your contribution! |
| @inline $_f!(::IndexLinear, A::AbstractVector, v, i::Int) = $f!(A, v, i) | ||
| @inline $_f!(::IndexLinear, A::AbstractArray, v, i::Int) = $f!(A, v, i) | ||
| @inline function $_f!(::IndexLinear, A::AbstractArray, v, I::Vararg{Int, M}) where {M} | ||
| @inline function $_f!( |
There was a problem hiding this comment.
Just out of curiosity, this looks quite bad to me, and is basically reverting back to JuliaFormatter. Is this what we want?
There was a problem hiding this comment.
I agree this one isn't good, and not what I would want in the ideal case. It's an unfortunate side affect of the slightly hacky way we are doing things in https://github.com/ITensor/ITensorFormatter.jl right now, where indeed it is caused by JuliaFormatter.
Basically the goal of ITensorFormatter.jl is to format in a way that is consistent with Runic.jl (i.e. the final output should be a fixed point of Runic formatting) but be more opinionated about formatting/organizing imports and dealing with line wraps in a more automated way. So the ideal would be the following:
- In general format like Runic.
- Sort lists of explicit imports, and have lists of explicit imports organized into a minimal set of lines that wrap around at the standard 92 character limit (in contrast with JuliaFormatter which splits them each into separate lines for long lists of imports, and Runic which doesn't enforce any wrapping, and also neither one sorts them).
- In general bias towards longer lines of code but enforce the 92 character limit.
The reason is that both for my own code and when reviewing code, 2. and 3. have been annoying to try to enforce without an automated solution in the formatter, and I think Runic is too unopinionated about those issues.
Basically neither Runic nor JuliaFormatter gives us a good solution to 2. and 3., so for 2. (import sorting) we rolled our own solution in ITensorFormatter.jl based on code for that defined in LanguageServer.jl. The strategy for 3. is to use a pass of JuliaFormatter with join_lines_based_on_source set to true to try to preserve longer lines where they exist in the code already but add line breaks if they go beyond 92 characters, this case seemed to be too aggressive with splitting into more lines and I'm not sure why.
Modulo weird cases like this, I think the formatting that ITensorFormatter is good enough as a first draft so the goal right now for me is to upgrade from Runic to ITensorFormatter in ITensorActions and ITensorPkgSkeleton, and then later on once that is set up we can try to tweak the rules or role our own solutions to fix weird cases like this. It is definitely unfortunate that ITensorFormatter uses both Runic and JuliaFormatter, again the goal was to role something that is good enough for now. Eventually the goal would be to just use Runic as the primary formatter with some more opinionated fixes on top of it for automated but not overzealous linebreaks, say written with custom code based on JuliaSyntax.
There was a problem hiding this comment.
See: #101 where I've fixed up some of these uglier formatting changes. I think basically what happened was that when certain types of code runs over the margins, even though join_lines_based_on_source = true, JuliaFormatter biases towards splitting up each argument into separate lines. At least when that happens, we can manually pack arguments onto longer lines and as long as none of them run past the margins it doesn't revert the manual changes, so it seems like the current setup works well in terms of a balance of automation and allowing some manual fixing.
2 participants
No description provided.