First pass in inlining the necessary bits from a.el #29

plexus · 2021-08-12T14:40:18Z

At this point only the tests still contain two invocations of a-equal. Unfortunately inlining that one would mean copying over much of a.el, and there is no close equivalent immediately available in Emacs.

plexus · 2021-08-12T15:01:51Z

Somewhat frustrating that map-insert introduces duplicates, even though the docstring claims it doesn't

(map-insert '((:node-type . :symbol) (:position . 14) (:form . "bar") (:value . bar)) :form "123")
;;=>
((:form . "123") (:node-type . :symbol) (:position . 14) (:form . "bar") (:value . bar))

bbatsov · 2021-08-13T07:46:19Z

Perhaps @NicolasPetton can help with this?

At this point only the tests still contain two invocations of a-equal.

That's fine, as now a.el can be a dev dep, instead of a runtime dep.

basil-conto · 2021-08-25T23:07:10Z

Somewhat frustrating that map-insert introduces duplicates, even though the docstring claims it doesn't

AFAICT the docstring explicitly allows for duplicates in the case of alists, since map-insert is non-destructive and alist lookup order is well-defined, with earlier elements shadowing later ones:

map-insert is a compiled Lisp function in `map.el'.

(map-insert MAP KEY VALUE)

  Probably introduced at or before Emacs version 27.1.

Return a new map like MAP except that it associates KEY with VALUE.
This does not modify MAP.
If you want to insert an element in place, use `map-put!'.
The default implementation defaults to `map-copy' and `map-put!'.

This is a generic function.

Implementations:

((map list) key value) in `map.el'.

Cons KEY and VALUE to the front of MAP.

Note in particular the alist specialiser's docstring.

basil-conto

Thanks, just a few tiny notes from me in passing :).

basil-conto · 2021-08-25T23:16:33Z

parseclj.el

+(defun parseclj-alist (&rest kvs)
+  "Create an association list from the given keys and values KVS.
+Arguments are simply provided in sequence, rather than as lists or cons cells.
+For example: (a-alist :foo 123 :bar 456)"


Should the example mention parseclj-alist instead of a-alist?

Yes, fixed!

basil-conto · 2021-08-25T23:20:56Z

parseclj.el

+  "Create an association list from the given keys and values KVS.
+Arguments are simply provided in sequence, rather than as lists or cons cells.
+For example: (a-alist :foo 123 :bar 456)"
+  (mapcar (lambda (kv) (cons (car kv) (cadr kv))) (seq-partition kvs 2)))


FWIW, since map.el version 2.0, which can be installed from GNU ELPA and comes bundled with Emacs 27, one can also say (map-into kvs 'alist).

Thanks, good suggestion. I've added them as a comment for now. Would it be enough to declare (map "2") as a Package-Requires for this to generally work out of the box?

Yeah, that's enough.

basil-conto · 2021-08-25T23:22:33Z

parseclj.el

+hash table is created without extra storage space, so with a size
+equal to amount of key-value pairs, since it is assumed to be
+treated as immutable.
+For example: (a-hash-table :foo 123 :bar 456)"


Ditto re: a-hash-table/parseclj-hash-table.

basil-conto · 2021-08-25T23:29:38Z

parseclj.el

+  (let* ((kv-pairs (seq-partition kvs 2))
+         (hash-map (make-hash-table :test 'equal :size (length kv-pairs))))
+    (seq-do (lambda (pair)
+              (puthash (car pair) (cadr pair) hash-map))
+            kv-pairs)
+    hash-map))


Ditto re: (map-into kvs 'hash-table).

basil-conto · 2021-08-25T23:31:36Z

parseclj.el

+    hash-map))
+
+(defun parseclj-alist-assoc (coll k v)
+  "Like parseclj-alist-assoc but actually works as advertised, not


Recursive docstring? :)

I'm guessing a search-and-replace gone wrong. I've rewritten the docstring.

basil-conto · 2021-08-25T23:43:43Z

parseclj.el

+  (if (map-contains-key coll k)
+      (mapcar (lambda (entry)
+                (if (equal (car entry) k)
+                    (cons k v)


Maybe this is desirable, but FWIW that (cons k v) will replace all equal duplicates of k with eq duplicates of k.

If that is undesirable, one can write (cons (car entry) v) instead, thus preserving the original key.

Not something I thought about much since the whole point of value semantics is that these would be indistinguishable, but in case they are not I do think it's more useful for the newer k to replace the old.

plexus · 2021-08-30T12:37:20Z

I've incorporated the feedback, and also actually dropped the a.el from the Package-Requires and bumped the Version from 0.1.0 to 0.2.0 (although maybe it should just be 1.0).

I hope that's good, let me know if folks needs anything more from me.

bbatsov · 2021-08-30T12:52:56Z

Thanks, Arne! I think we're good to go here, although I also think 1.0 reflects better the state of the project at this point.

I hope that's good, let me know if folks needs anything more from me.

We need the similar changes done for parseedn, but I guess there it should be simpler as the project is smaller.

Necessary for inclusing in ELPA. See also clojure-emacs/parseclj#29

bbatsov · 2021-09-22T06:12:44Z

I think we're good to go here. Arne would you like to merge this or you're fine with me doing it and cutting the new release?

plexus · 2021-09-22T06:43:34Z

Please feel free to do the honors!

Necessary for inclusing in ELPA. See also clojure-emacs/parseclj#29

plexus added 3 commits August 12, 2021 16:37

First pass in inlining the necessary bits from a.el

ff1a3c7

Use map-elt instead of parseclj-alist-get

90d7bea

Use map-contains-key

6824f8e

basil-conto reviewed Aug 25, 2021

View reviewed changes

plexus added 2 commits August 30, 2021 14:17

Update docstring and metion Emacs 27 alternative

ace3b2d

Drop a.el dependency, bump copyright year, bump version

8a4a830

plexus added a commit to plexus/parseedn that referenced this pull request Sep 2, 2021

Remove the a.el dependency

602c9b5

Necessary for inclusing in ELPA. See also clojure-emacs/parseclj#29

plexus mentioned this pull request Sep 2, 2021

Remove the a.el dependency clojure-emacs/parseedn#9

Merged

bbatsov mentioned this pull request Sep 21, 2021

Doc: suggest in advance using stable releases clojure-emacs/cider#3053

Closed

bbatsov merged commit 089160a into master Sep 27, 2021

bbatsov deleted the remove-a-dep branch September 27, 2021 06:20

bbatsov pushed a commit to clojure-emacs/parseedn that referenced this pull request Sep 27, 2021

Remove the a.el dependency

3f93aab

Necessary for inclusing in ELPA. See also clojure-emacs/parseclj#29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

First pass in inlining the necessary bits from a.el #29

First pass in inlining the necessary bits from a.el #29

plexus commented Aug 12, 2021

plexus commented Aug 12, 2021

bbatsov commented Aug 13, 2021

basil-conto commented Aug 25, 2021

basil-conto left a comment •

edited

Loading

basil-conto Aug 25, 2021

plexus Aug 30, 2021

basil-conto Aug 25, 2021

plexus Aug 30, 2021

bbatsov Aug 30, 2021

basil-conto Aug 25, 2021

plexus Aug 30, 2021

basil-conto Aug 25, 2021

basil-conto Aug 25, 2021

plexus Aug 30, 2021

basil-conto Aug 25, 2021

plexus Aug 30, 2021

plexus commented Aug 30, 2021

bbatsov commented Aug 30, 2021

bbatsov commented Sep 22, 2021

plexus commented Sep 22, 2021

First pass in inlining the necessary bits from a.el #29

First pass in inlining the necessary bits from a.el #29

Conversation

plexus commented Aug 12, 2021

plexus commented Aug 12, 2021

bbatsov commented Aug 13, 2021

basil-conto commented Aug 25, 2021

basil-conto left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

plexus commented Aug 30, 2021

bbatsov commented Aug 30, 2021

bbatsov commented Sep 22, 2021

plexus commented Sep 22, 2021

basil-conto left a comment •

edited

Loading