Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Allow block indentation inside of parentheticals #152

Merged
merged 2 commits into from
Mar 23, 2021

Conversation

hexane360
Copy link
Contributor

I've tweaked the indentation code slightly, so that blocks are indented correctly inside of parentheses and brackets. I've also added a couple of test cases.

Here's some examples:

block_in_func(begin
                  arg = 2,
                  begin
                      test
                  end
              end)

var = func(arg1, arg2, begin
               arg3
           end)

begin
    var = [aligned,  # works with brackets and parens
           arg,
           begin
               statement
               arg
           end,
           arg
           ]
    func(
        indented,
        1 +  # works with hanging operators
            begin
                arg
            end,
        arg
    )
end

This should fix a large portion (though not all) of the complaints in #11.

@hexane360
Copy link
Contributor Author

BTW, here's the indentation given for some of the test cases in issue #11:

function1(a, b, c # correct, alignment style
          d, e, f)
function2( # correct, indent style
    a, b, c
    d, e, f)
for i in Float64[1, 2, 3, 4 #correct, alignment style
                 5, 6, 7, 8]
end
for i in Float64[ #correct, indent style
    1, 2, 3, 4
    5, 6, 7, 8]
end
a = function3(function () #correct, alignment style
                  return 1
              end)
a = function4( #correct, indent style
    function ()
        return 1
    end
)
longfunctionname(;  # I don't know if we should special-case this
                 hello=nothing,
                 world=nothing,
                 )
longfunctionname(
    hello=nothing,
    world=nothing,
)
hl = Highlighter((h,data,i,j)->begin
                     id = div(i-1, 3)
                     sim_id = id*num_cols + j
                     if sims_status[sim_id] == 1
                         return crayon"black bg:green"
                     else
                         return crayon"bg:yellow"
                     end
                 end)
f(a, b,
  c) do i
      sin(i)  # i agree this could still look better
end

And issue #111:

f(
    map(1:3) do x
        x
    end
)

@ronisbr
Copy link
Contributor

ronisbr commented Feb 19, 2021

This is awesome!

@tpapp @non-Jedi can you please take a look at this PR? This modification is very important to julia-emacs. It fixes some annoying issues.

Copy link
Collaborator

@tpapp tpapp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice PR, just requested a minor style change.

julia-mode.el Outdated
(backward-up-list)
(1+ (current-column)))
(error (current-indentation)))
(widen))))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of (widen), perhaps wrap the narrowing in a (save-restriction ...) block?

@tpapp
Copy link
Collaborator

tpapp commented Feb 19, 2021

@hexane360: thanks for this very nice PR, and apologies for the late review. I will wait for a review from @non-Jedi (if he has the time) and them I am happy to merge, I just suggested one minor change.

@ronisbr: thanks for the ping.

@ronisbr
Copy link
Contributor

ronisbr commented Feb 19, 2021

@tpapp thanks for the quick response! :)

@ronisbr
Copy link
Contributor

ronisbr commented Mar 21, 2021

@tpapp is it possible to accept this without that proposed modification and then fix it if any problem occurs? Is there any big problem with (widen)?

@hexane360
Copy link
Contributor Author

hexane360 commented Mar 21, 2021

I just went ahead and wrote up the style change. This should be ready to merge (or rollup merge) now.

@tpapp
Copy link
Collaborator

tpapp commented Mar 21, 2021

@hexane360: thanks.

@non-Jedi, @giordano, @FelipeLema: if any of you have time for a review, it would be appreciated, then I would merge.

Copy link
Contributor

@FelipeLema FelipeLema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good (Y)

Copy link

@giordano giordano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@tpapp tpapp merged commit fe6f6f7 into JuliaEditorSupport:master Mar 23, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants