Fix incorrect indentation of namespaced map - #513
Conversation
| (comment-normalize-vars) | ||
| (comment-forward (point-max)) | ||
| (looking-at-p "\\^\\|#[[:alpha:]]")) | ||
| (looking-at-p "\\^\\|#:?:?[[:alpha:]]")) |
There was a problem hiding this comment.
(it seems to me like 0-2 :, but I think this can be expressed better with a different qualifier). I also don't see tests with ::.
There was a problem hiding this comment.
At least not in the first test.
There was a problem hiding this comment.
What's this supposed to match?
^metadata or
#reader-macro or
#:ns-unqualified-reader-macro or
#::namespaced-qualified-reader-macro
Yes, it can be expressed as #:{0,2}
The test case uses #::ns, but I did not write cases for each permutation.
There was a problem hiding this comment.
Btw, there's a small problem with the regexp now - something like ^:: will also match. Probably it should be tightened.
There was a problem hiding this comment.
And I'd really use :\{0,2\} instead of :?:?.
| (clojure-mode) | ||
| (clojure-backward-logical-sexp 1) | ||
| (should (looking-at-p "#:name/space{:k v}")) | ||
| (insert " #::ns {:k v}") |
There was a problem hiding this comment.
Why is there are space here between the namespace and the map?
There was a problem hiding this comment.
Both #:ns {:k v} and #:ns{:k v} are valid syntax and has the same meaning. Also, I have not seen any guideline that prefers one style over the other.
There was a problem hiding this comment.
Got it. I thought usually it was the form without the space.
|
Apart from the failing build (which might be some random Travis failure looking at it), this change needs a changelog entry. |
5e8698b to
e3a852b
Compare
|
Updated changelog. The CI failure looks like build environment related. I'm not too familiar with it though.. |
|
You need to rebase on top of |
e3a852b to
db0ba99
Compare
|
Done, checks passed. |
|
|
||
| (check-indentation cond-indentation-with-namespaced-map | ||
| " | ||
| (cond-> #:a{:b 1} |
There was a problem hiding this comment.
Don't forget to add here a test with ::.
|
Great! Please, address the small feedback I've added and we're good to go. |
|
@poernahi ping :-) |
|
@poernahi Ping 2 :-) |
|
@anthonygalea Can I ask you to pick this one up and just update the tests for buttercup? |
|
Superseded by #533. |
Fix Incorrect Indentation of Namespaced Map Literal
Please refer to #511 for bug description.
M-x checkdocand fixed any warnings in the code you've written.