Added ipcalc-org to be used within babel blocks #9
Conversation
|
Hello @alinmear Thank you for the kind words and thank you for creating this PR. About the change, it seems to me, at a quick glance, that your Could you not just wrap the call to |
|
hi @dotemacs; sorry for the bad style of my initial pr; just hacked it quickly together. I refactored the changes as you suggested. Thx again for your great work. |
There was a problem hiding this comment.
Hi @alinmear
Thank you for the changes.
Just for my own understanding, you've extracted the code from the previous approach with ipcalc-org function and teased out ipcalc-calc as a separate function, that can be called stand alone, but also from ipcalc, right?
If the above is true, then great, thank you for that.
I'd be happy to merge it in, once you address some of the minor style comments I've left.
Also, when you finish with that, maybe update the git commit message to explain why this work was done, for anybody who'll come after to have the better context.
Is that OK with you?
Thanks
| (4th-octet (substring binary 24 32)) | ||
| (octets (mapcar | ||
| 'ipcalc-bin-to-int | ||
| `(,1st-octet ,2nd-octet ,3rd-octet ,4th-octet)))) |
There was a problem hiding this comment.
Can you confirm that the lines 131-138 aren't being touched?
I'm asking for this because I'm not sure if this is just GitHub's UI playing tricks or if you've indent this?
| "IP calculator for given IP/CIDR." | ||
| (interactive "sIP/CIDR: ") | ||
| (let* ((ip (car (split-string ip/cidr "/"))) | ||
| (defun ipcalc-calc (ip/cidr) |
There was a problem hiding this comment.
I take it that with this function you're hoping that it'll be available to be called as stand alone, as discussed previously?
Maybe add the magic comment (;;;###autoload) to it?
See: https://www.gnu.org/software/emacs/manual/html_node/elisp/Autoload.html
| "Call IPcalc as function without separate buffer." | ||
| (interactive) | ||
| (let* ( | ||
| (ip (car (split-string ip/cidr "/"))) |
There was a problem hiding this comment.
Move this line up so that the paren above is not left dangling
| (kill-buffer buffer)) | ||
| (pop-to-buffer buffer) | ||
| (insert | ||
| ) |
There was a problem hiding this comment.
The same here, don't leave the paren danging, but move it up with the others on the previous line.
| (interactive "sIP/CIDR: ") | ||
| (let* ( | ||
| (buffer "*ipcalc*") | ||
| ) |
There was a problem hiding this comment.
Instead of lines 179-181,try this:
| ) | |
| (let* ((buffer "*ipcalc*")) |
|
|
||
| (pop-to-buffer buffer) | ||
| (insert (ipcalc-calc ip/cidr)) | ||
| )) |
There was a problem hiding this comment.
The same here, move those two parens on the previous line.
| (insert (ipcalc-calc ip/cidr)) | ||
| )) | ||
|
|
||
| (provide 'ipcalc-calc) |
There was a problem hiding this comment.
About this line, you don't need it.
You only write provide to say "Hey, my package foo is done, here it is."
So you'd want to drop that line and then see my comment above about the "magic comment". That'll resolve what you're trying to do here.
| (let* ( | ||
| (buffer "*ipcalc*") | ||
| ) | ||
| (if (get-buffer buffer) |
There was a problem hiding this comment.
Instead of if, can when be used?
|
Hey @alinmear I pushed a formatting change into this branch: https://github.com/alinmear/ipcalc.el/compare/2720f7e3e662e04e195f8338b81a499cf321296a...dotemacs:standalone-calc?expand=1 (I've subsequently added some more commits, but do ignore them for now.) But let me ask you, couldn't this package be used as is, without your change, in org-mode? As in, you could call What benefit does this extracted function Thanks |
It's pretty handy to use your solution within org files