Skip to content
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

command for macroexpanding the expression #1

Closed
wants to merge 1 commit into from
Closed

command for macroexpanding the expression #1

wants to merge 1 commit into from

Conversation

alexander-yakushev
Copy link
Member

I mostly stole this from SLIME because I feel a critical lack of it in nREPL.

Having little experience with elisp there could be mistakes, but I'd like to give this function a spin.

@kingtim
Copy link
Member

kingtim commented Jun 12, 2012

Looks good, thank you!

@kingtim kingtim closed this Jun 12, 2012
@kingtim
Copy link
Member

kingtim commented Jun 14, 2012

Hi Alexander,
I just pushed some changes to the macroexpansion handling. I cloned most of the popup handling from slime, so that pressing 'q' in a popup buffer will dismiss it. I also refactored the handlers so that the buffer can be read-only after the expansion. Also, I changed the pprint macroexpansion so that only the stdout is emitted, since pprint sends it output to stdout, so you don't get that extra "nil" in the output. I tested all of this locally and it seems to be working for me.

When you have some time, please give it a spin and let me know what you think?
I am also thinking of adding a C-c M-m which will do macroexpand, and change C-c C-m to do macroexpand-1, What do you think of that?

Again, thanks for the pull request!

@alexander-yakushev
Copy link
Member Author

Hello Tim,
It is a great idea of yours to make buffers read-only and closeable by pressing "q". Changing C-c C-m to macroexpand-1 and introducing C-c M-m is also good (AFAIK SLIME works exactly this way).

Though I can't get the changed macroexpansion working. First of all, in the nrepl-macroexpand-last-sexp there is a form variable which should be substituted by (format command expr). But even with this change I still get the following message:

error in process filter: Wrong number of arguments: ((nrepl-input-start-mark nrepl-prompt-start-mark t) (buffer value) (with-current-buffer buffer (let ((inhibit-read-only t) (buffer-undo-list t)) (erase-buffer) (insert (format "%s" value)) (goto-char (point-min)) (indent-sexp) (font-lock-fontify-buffer)))), 1

I can't find what's wrong. Can you take a look?

@kingtim
Copy link
Member

kingtim commented Jun 14, 2012

Sorry about that, I hadn't pushed my latest changes. Should be working now.

@alexander-yakushev
Copy link
Member Author

Thanks, it works great!

Though I found an issue with macroexpanding (it's not unique to the introduced functions, but also happens in the interactive REPL). When I macroexpand some macros here's what I receive in the Messages buffer:

error in process filter: let: Args out of range: 102, 5427
error in process filter: Args out of range: 102, 5427
error in process filter: cond: Cannot decode object: 1
error in process filter: Cannot decode object: 1

Trying to macroexpand the same expression from REPLy or CCW works fine. I thought maybe you've run into this kind of error?

@kingtim
Copy link
Member

kingtim commented Jun 15, 2012

That sounds like a bug in the bencode decoder. If you can give me a reproducible test case I can track it down.
Thanks!

@alexander-yakushev
Copy link
Member Author

It's hard to provide my exact reproducible case but here's another one for the time being. I don't know if they are quite connected, but...

(str \u2190)

Should just return a string with an Unicode character. It does for the REPLy but returns this in nrepl.el:

error in process filter: Args out of range: 100, 105

Typing this one yields the same:

(str "←")

The case I'm struggling with doesn't have any Unicode characters bit I currently can't minimize it down to the single problem.

@kingtim
Copy link
Member

kingtim commented Jun 20, 2012

I think I have tracked down the multibyte decoding issue and pushed a fix. Let me know if that fixes the above issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants