abo-abo / lispy

Short and sweet LISP editing
http://oremacs.com/lispy/
1.2k stars 132 forks source link

Implement lispy-isolate #535

Closed zaeph closed 4 years ago

zaeph commented 4 years ago

Hi there,

When I'm investigating a function, I like to narrow the buffer to it to make it easier to focus on it. However, when I'm in an edebug session, I find it easier to clone the buffer, and narrow to that function in that cloned buffer. It makes it easier to visit other functions in the file.

lispy-isolate is a proof-of-concept that does just that. The name's not great, but before going forward, I'd prefer to make sure that you haven't already implemented something similar. If not, I'd be willing to write something a little more fleshed out.

coveralls commented 4 years ago

Coverage Status

Coverage decreased (-0.2%) to 63.404% when pulling 1fbc1b6b2f745d9bea95cd82ed78309ea3a967c4 on zaeph:isolate into 8115d8f53d7b86c365880b772fdefae15c996adc on abo-abo:master.

abo-abo commented 4 years ago

Interesting. What's the advantage of cloning the buffer?

zaeph commented 4 years ago

Interesting. What's the advantage of cloning the buffer?

lispy-goto-symbol always finds the definition of the symbol in a real buffer. If you've narrowed the buffer and want to visit a symbol that is in the same file, but outside of the current restriction, the restriction will not be removed. This is especially bad when you're trying to find out which nested function is causing a problem, since you constantly have to go back and forth between the different levels of nesting.

I've found cloning-and-narrowing to be a good solution to that problem.

abo-abo commented 4 years ago

Could we instead have lispy-goto-symbol respect the current narrowing?

zaeph commented 4 years ago

Could we instead have lispy-goto-symbol respect the current narrowing? [emphasis added]

What do you mean by this? I see two ways to understand it:

  1. If we're narrowed to a def (or potentially to anything else), then narrow to def after lispy-goto-symbol, narrow.
  2. Warn the user that the symbol is outside the restriction.

Now, we could widen after lispy-goto-symbol, which I think would fit with the intention of users when running the command. Howeber, that does not address the workflow I've described above, since we'd still be in the main file-visiting buffer rather than an indirect clone.

abo-abo commented 4 years ago

OK. So you want to be able to use lispy-goto-symbol, but you don't want to be able to go back to your current narrowing.

Then we could do this:

This way, all users gain the potential benefit, without adding a new command. Would this work for you?

zaeph commented 4 years ago

Would this work for you?

It would be absolutely wonderful. Hadn't thought of using a history, thanks!

I have some experience with making a history of narrowing, and reverting to those. Generally, a local-variable to which you push and pop is enough.

Would you like me to take care of it?

abo-abo commented 4 years ago

Would you like me to take care of it?

Would be great. Thanks in advance.

zaeph commented 4 years ago

I'll schedule this for next week, then.

zaeph commented 4 years ago

PR is ready.

  • g (lispy-goto-symbol) removes the narrowing

  • b (lispy-back) can restore the point position and the narrowing after lispy-goto-symbol [emphasis added]

I've implemented both behaviours. I was iffy on the bold part because my first implementation wasn't robust, but reworking it convinced me that it was useful.

I've made the behaviour toggleable with lispy-back-restore-restriction (default to t).

abo-abo commented 4 years ago

Thanks, the code looks good. Some comments:

zaeph commented 4 years ago

Sure, I'm going to address your comments now.

zaeph commented 4 years ago
  • I'm not sure about which Emacs version supports pcase-let*. Maybe use cl-destructuring-bind or a simple let* instead?

Addressed in 1fbc1b6b2f745d9bea95cd82ed78309ea3a967c4.

  • I think lispy-goto and lispy-goto-local should work on the whole buffer, even if it's narrowed.

I'm not sure if I understand. I've only touched lispy-goto-symbol which I access with the M-.. Does that mean you'd want me to patch lispy-goto and lispy-goto-local similarly to what I've done with lispy-goto-symbol?

abo-abo commented 4 years ago

Thanks, I see now. I think it's fine to merge then. lispy-goto working on a narrowed buffer can be a separate issue.

zaeph commented 4 years ago

I'll try to play around with lispy-goto on my end to see if there's a need for it, and I'll open a PR if I feel it's a good idea. Thanks for the merge!