Skip to content

Align SciML Dict tuple pairs#999

Open
svchb wants to merge 2 commits into
JuliaEditorSupport:masterfrom
svchb:split/sciml-dict-tuple-pairs
Open

Align SciML Dict tuple pairs#999
svchb wants to merge 2 commits into
JuliaEditorSupport:masterfrom
svchb:split/sciml-dict-tuple-pairs

Conversation

@svchb

@svchb svchb commented May 28, 2026

Copy link
Copy Markdown
Contributor

Before:

algorithm_cases = Dict(
    "default search" => (),
    "with static scheduler" => (backend=StaticBackend(),),
    "with spline kernel" => (smoothing_length=1.1 *
                                              item_spacing,
                                smoothing_kernel=SplineKernel{2}())
)

After:

algorithm_cases = Dict(
    "default search"        => (),
    "with static scheduler" => (backend=StaticBackend(),),
    "with spline kernel"    => (smoothing_length=1.1 * item_spacing,
                                smoothing_kernel=SplineKernel{2}())
)

@svchb svchb marked this pull request as ready for review May 28, 2026 10:12
@svchb svchb mentioned this pull request May 28, 2026
@penelopeysm

Copy link
Copy Markdown
Member

This is another slightly nuanced one...

(1) It shouldn't join the 1.1 * item_spacing, unless join_lines_based_on_source = false. I think this is similar to previous examples we've discussed.

(2) I don't think there's any style where pair arrows are forced to align. There is the align_pair_arrow option, which you can set to true but this only aligns the pair arrows when there's pre-existing alignment. That is, if align_pair_arrow=true,

a => b
cccc => dddd

won't be converted to

a    => b
cccc => dddd

. However, if it's already in that latter format, then align_pair_arrow is supposed to make sure it doesn't revert to the former.

This does actually behave correctly (kind of) -- I've tweaked this input so that 1.1 * item_spacing is on the same line, and added a few more spaces to the default search line to enable the custom alignment heuristic

julia> s = """
       algorithm_cases = Dict(
           "default search"        => (),
           "with static scheduler" => (backend=StaticBackend(),),
           "with spline kernel" => (smoothing_length=1.1 * item_spacing,
                                       smoothing_kernel=SplineKernel{2}())
       )
       """
"algorithm_cases = Dict(\n    \"default search\"        => (),\n    \"with static scheduler\" => (backend=StaticBackend(),),\n    \"with spline kernel\" => (smoothing_length=1.1 * item_spacing,\n                                smoothing_kernel=SplineKernel{2}())\n)\n"

julia> s |> println
algorithm_cases = Dict(
    "default search"        => (),
    "with static scheduler" => (backend=StaticBackend(),),
    "with spline kernel" => (smoothing_length=1.1 * item_spacing,
                                smoothing_kernel=SplineKernel{2}())
)

julia> format_text(s, SciMLStyle(); align_pair_arrow=true) |> println
algorithm_cases = Dict(
    "default search"        => (),
    "with static scheduler" => (backend = StaticBackend(),),
    "with spline kernel"    => (smoothing_length = 1.1 * item_spacing,
    smoothing_kernel = SplineKernel{2}())
)

So the alignment is behaving correctly, although the indentation of smoothing_kernel is probably not right -- it should be indented one more level.

All in all, I think there are some bugs here, but I'm not sure they're the same bugs that the PR tries to fix.

@penelopeysm

Copy link
Copy Markdown
Member

There could also be a flag that says "yes really force the alignment even if it's not already aligned to begin with" -- that'd be a separate config option though that would apply across all styles.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants