[Fix #471] Added support for tagged maps #472
Conversation
|
CI failed on a test, on only a couple versions, and I don't think I changed anything that would affect it. Do you have any suggestions as to why it broke? |
CHANGELOG.md
Outdated
| @@ -10,6 +10,7 @@ | |||
| * New interactive command `clojure-cycle-not`. | |||
| * New defcustom `clojure-comment-regexp` for font-locking `#_` or `#_` AND `(comment)` sexps. | |||
| * [#459](https://github.com/clojure-emacs/clojure-mode/issues/459): Add font-locking for new built-ins added in Clojure 1.9. | |||
| * [#471](https://github.com/clojure-emacs/clojure-mode/pull/471): Support tagged maps, new in Clojure 1.9 | |||
There was a problem hiding this comment.
I think the link is not correct - looks like a pull request link, not an issue link. Also, I guess you meant to say this adds support for Paredit, right?
This sentence should end with a ., btw.
There was a problem hiding this comment.
It doesn't add support for paredit, it fixes the existing support.
There was a problem hiding this comment.
Yeah, yeah. I meant that what you wrote could be interpreted as the addition of some broader set of functionality for namespaced maps, so I wanted the entry to be more specific.
Btw, @benedekfazekas, that's a nice idea for a simple refactoring - making a map namespaced/unnamespaced.
clojure-mode.el
Outdated
| @@ -521,7 +521,8 @@ replacement for `cljr-expand-let`." | |||
| (setq-local clojure-expected-ns-function #'clojure-expected-ns) | |||
| (setq-local parse-sexp-ignore-comments t) | |||
| (setq-local prettify-symbols-alist clojure--prettify-symbols-alist) | |||
| (setq-local open-paren-in-column-0-is-defun-start nil)) | |||
| (setq-local open-paren-in-column-0-is-defun-start nil) | |||
There was a problem hiding this comment.
Not sure why you changed this, probably accidentally.
clojure-mode.el
Outdated
| @@ -733,7 +734,9 @@ definition of 'macros': URL `http://git.io/vRGLD'.") | |||
| (concat "[^" clojure--sym-forbidden-1st-chars "][^" clojure--sym-forbidden-rest-chars "]*") | |||
| "A regexp matching a Clojure symbol or namespace alias. | |||
| Matches the rule `clojure--sym-forbidden-1st-chars' followed by | |||
| any number of matches of `clojure--sym-forbidden-rest-chars'.")) | |||
| any number of matches of `clojure--sym-forbidden-rest-chars'.") | |||
| (defconst clojure--collection-tag-regexp "#\\(::[a-zA-Z0-9._-]*\\|:?\\([a-zA-Z0-9._-]+/\\)?[a-zA-Z0-9._-]+\\)" | |||
There was a problem hiding this comment.
I'd move this before its usage, and remove that leading indentation.
clojure-mode.el
Outdated
| any number of matches of `clojure--sym-forbidden-rest-chars'.")) | ||
| any number of matches of `clojure--sym-forbidden-rest-chars'.") | ||
| (defconst clojure--collection-tag-regexp "#\\(::[a-zA-Z0-9._-]*\\|:?\\([a-zA-Z0-9._-]+/\\)?[a-zA-Z0-9._-]+\\)" | ||
| "Allowed strings that can come before a collection literal (e.g. '[]' or '{}'), as reader macro tags. This includes #fully.qualified/my-ns[:kw val] and #::my-ns{:kw val} as of Clojure 1.9.")) |
There was a problem hiding this comment.
The first sentence should be on a life of its own.
| (with-temp-buffer | ||
| (clojure-mode) | ||
| (insert tag) | ||
| (should-not (clojure-no-space-after-tag nil ?{)))) |
There was a problem hiding this comment.
Shouldn't you add some comparison in that should-not?
| (with-temp-buffer | ||
| (clojure-mode) | ||
| (insert tag) | ||
| (should (clojure-no-space-after-tag nil ?{))))) |
|
It’s a boolean function. I guess I could do (= t), but that seems redundant.
…--
Evan Moses
[email protected]
On Mar 11, 2018, at 6:08 PM, Bozhidar Batsov ***@***.***> wrote:
@bbatsov commented on this pull request.
In test/clojure-mode-syntax-test.el:
> @@ -78,6 +78,20 @@
(backward-prefix-chars)
(should (bobp)))))
+
+(ert-deftest clojure-allowed-collection-tags ()
+ (dolist (tag '("#::ns" "#:ns" "#ns" "#:f.q/ns" "#f.q/ns" "#::"))
+ (with-temp-buffer
+ (clojure-mode)
+ (insert tag)
+ (should-not (clojure-no-space-after-tag nil ?{))))
Shouldn't you add some comparison in that should-not?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
Got it. I guess we should have named it |
|
Thanks for working on this! I really appreciate it! 🙇 |
Updated
clojure-no-space-after-tagto support tagged maps with autoresolved namespaces, such as#::{:foo "bar"}and#::my-ns{:foo "bar"}. This is new reader syntax added in Clojure 1.9Before submitting a PR mark the checkboxes for the items you've done (if you
think a checkbox does not apply, then leave it unchecked):
M-x checkdocand fixed any warnings in the code you've written.Thanks!