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

Align a nested map #360

Closed
rfkm opened this issue Jan 15, 2016 · 9 comments
Closed

Align a nested map #360

rfkm opened this issue Jan 15, 2016 · 9 comments
Assignees
Labels

Comments

@rfkm
Copy link

rfkm commented Jan 15, 2016

Run clojure-align against the following code:

{:a {:a :a
     :bbbb :b}
 :bbbb :b}

Then you'll get:

{:a    {:a :a
     :bbbb :b}
 :bbbb :b}

Is this intended behavior?
align-cljlet can do this in one step:

{:a    {:a    :a
        :bbbb :b}
 :bbbb :b}
@bbatsov
Copy link
Member

bbatsov commented Jan 15, 2016

Yeah, there's room for improvement here. @Malabarba will you, please, look into this?

@bbatsov bbatsov added the bug label Jan 15, 2016
@Malabarba
Copy link
Member

Of course!

@Malabarba Malabarba self-assigned this Jan 16, 2016
@rfkm
Copy link
Author

rfkm commented Jan 16, 2016

Sorry, I lied.
align-cljlet can not do this in one step.
Actually, it requires two steps:

{:a {:a :a
     :bbbb :b}
 :bbbb :b}

{:a    {:a :a
        :bbbb :b}
 :bbbb :b}

{:a    {:a    :a
        :bbbb :b}
 :bbbb :b}

This behavior is acceptable to me. (Of course, it would be nice if the recursive version is optionally available.)
So, the only problem is clojure-align doesn't indent after alignment?

@Malabarba
Copy link
Member

So, the only problem is clojure-align doesn't indent after alignment?

Yes, exactly.

Malabarba added a commit that referenced this issue Jan 16, 2016
@rfkm
Copy link
Author

rfkm commented Jan 16, 2016

@Malabarba
Great, thanks! I've confirmed the fix works.

However, I've found another problem...

Run clojure-align against the following map:

{:a {:a :a
     :aaaaaaaaaaaaaaaaaaaaaa :a}
 :b {:a :a
     :aa :a}}

Then:

{:a {:a                      :a
     :aaaaaaaaaaaaaaaaaaaaaa :a}
 :b {:a :a  ; <-
     :aa :a}}

It works with the following map:

{:a {:a :a
     :aaaaaaaaaaaa :a}  ; <- decrease the number of `a`
 :b {:a :a
     :aa :a}}

Probably, end passed to clojure--find-sexp-to-align should be a marker?

@bbatsov
Copy link
Member

bbatsov commented Jan 16, 2016

Run clojure-align against the following map:

Btw, it's way more convenient to use TAB to align. In my mind this functionality was always intended to be used like this (smarter indentation versus the needed to realign manually with the explicit command).

@rfkm
Copy link
Author

rfkm commented Jan 16, 2016

Yeah, the automatic mode is very convenient, but some people seem not to prefer such a code style, so I hesitate to enable it globally.
Maybe I'll use it for my personal project :)

@Malabarba
Copy link
Member

Probably, end passed to clojure--find-sexp-to-align should be a marker?

Good catch! :-)

@rfkm
Copy link
Author

rfkm commented Jan 17, 2016

@Malabarba Thanks a lot!

bbatsov pushed a commit that referenced this issue Mar 21, 2020
This prevents issues when aligning improperly indented code. Before, the region
would be aligned while retaining the improper indentation. The follow-up
indentation step would then break the alignment that was just performed.

With this commit, we switch the order by indenting first then aligning. This
retains compatibility with the test cases in #360.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants