-
Notifications
You must be signed in to change notification settings - Fork 137
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
Implement -cond-> and -cond->> macros. #349
base: master
Are you sure you want to change the base?
Conversation
Hi! First of all, thanks for your work on this. :) There seems to be a long-standing issue with the CI-tests, which is why the checks are failing. I haven't been active in maintaining this repo for quite some time. Sorry about that. Now, I have a few comments:
Also, I would prefer if
I might have read that last one too quickly, so do correct me if I am wrong. I haven't enough Emacs Lisp knowledge to know if Cheers! |
Hi,
I wasn't aware of these documenting features, dev/examples.el is updated now. As for the differences from Clojure's definitions,
Indeed, there was a mistake in my examples, it must be correct now.
This came as a surprise when i was testing. I tested a variable with the same name as the internally bound one, trying to see if there was a bug or an error, and this showed up. So I changed the symbol to
Just tested a version with (Also, I've completely mistyped last commit's message, sorry |
Using |
I'll look into it! :) Just out of curiosity, I tested some pieces of code, it would appear that these macros don't overshadow each other. Some of these lines were made complex on purpose. (-cond->> '(10 20 30 40 50 60)
(listp it) (--map (/ it 2))
(numberp it) (--filter (--> it (% it 2) (= it 0)))
(numberp (car it)) (--reduce (format "%s-%d" acc it))) ;; => "5-10-15-20-25-30"
(--map (-cond-> it
(numberp it) (/ 2)
(odd? it) (number-to-string))
'(10 20 30 40 50)) ;; => '("5" 10 "15" 20 "25")
(--> "abc"
(-cond-> it
t (concat "def" "ghi")
(string= it "abc") it
(string= it "abcdefghi") (split-string "" t)) ;; => '("a" "b" "c" "d" "e" "f" "g" "h" "i")
(--reduce-from (concat acc it) "" it)) ;; => "abcdefghi" Unfortunately, this is no exhaustive test of all available macros, and breaks uniformity of available macros. |
Use `dash--match-make-source-symbol` to make different, unused symbol every time. Apparently, symbol does not need to be interned, Just consistently evaluated.
Just committed using I guess this leaves for two options for anaphoric versions of this macro, one with (-cond--> '(10 20)
t (--map (/ it 2) it)
(= 1 1) (setf (car it) 11) ;; => '(11 10) and another following the last attempt, exposing the symbol |
I don't think |
I see. |
I'm glad to hear that. :) It seems the example generating code needs to be run another time, since the examples in the readme are outdated, but otherwise this looks good to me. |
I wasn't sure if i should commit the generated README. |
Well done. In order for this code to be allowed into the ELPA repo, all contributors must sign a copyright assignment. basil-conto said:
|
Sent! 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this! See some comments below.
All symbols that you normally refer to while using Emacs, such as the Since interning symbols by default happens in the global See more at |
@basil-conto |
- Use two pops and a push to build steps from clauses, using built-in functions only - Favor let over -let - If no clauses, return expression unchanged
Hi.
This is the implementation I mentioned in #330.
My main difficulty when porting this from Clojure's definition was regarding
make-symbol
vs.intern
.When using make-symbol, I ran into the same problem described here/ that is, when calling the macro, only the last expression would be tested and run.
Therefore I decided for
intern
. In my tests it did not interfere with other variables, but more tests are required.Hope this code fits with the package.
Also, some feedback would be appreciated, since this is my first PR.