justinbarclay / parinfer-rust-mode

Simplifying how you write Lisp
https://shaunlebron.github.io/parinfer/
GNU General Public License v3.0
227 stars 16 forks source link

run parinfer only when actually viewing buffer? #18

Closed andreyorst closed 4 years ago

andreyorst commented 4 years ago

I use desktop mode, and working on a big project right now that has tons of files, that do not satisfy Parinfer rules. Each time I launch Emacs, desktop restores these buffers, and Parinfer asks for each one if it should correct indentation. All these buffers are in background so I'd like to defer this check until I actually view those.

Another thing is when I'm loading all files in project with CIDER parinfer asks for each loaded file if it should fix it, when there are some hanging parentheses. I don't even see these buffers, so it is another place where deferring this check would be nice quality of life improvement.

It should be possible to detect visibility by calling get-buffer-window and defer Parinfer, although I'm not sure how to trigger Parinfer again when buffer will become visible.

Another thought is that not parsing background buffers may improve performance, but I'm not sure if this is really true, as parinfer should reparse on change?

This is minor enhancement, and not a bug that bothers me much, so if this is hard to do or impossible, this can be leaved as is.

justinbarclay commented 4 years ago

Yeah thats a good idea, I was thinking about some sort of system with a similar approach specified here.

andreyorst commented 4 years ago

Hm, enabling after first change may be trickier than enabling on buffer being viewed, because there may be another automatic change events like file reverting even if biffer in background, from magit for example.

justinbarclay commented 4 years ago

I've thought about this feauture over the weekend. I can see the annoyance in the prompt feature. If you open up 10 buffers at a time getting 10 different prompts is going to be frustrating.

However, I have tried to keep the customization in parinfer-rust-mode fairly bare-bones because I want to maintain as little code as possible. There are exceptions to this however where I try to maintain a minimal feature parity with other implementations (primarily Atom). As such, I think making parinfer-rust-mode smart or clever when it runs well outside of the Parinfer spec.

I am willing to add in a lesser system of hooks to so that others may tweak Parinfer's behavior. For instance, I could add a parinfer-rust-before-change-hook and that could allow someone to write a function that detects if the buffer it is running in is the active buffer and enable or disable parinfer-rust-mode.

With that said, parinfer-rust-mode does not manage parens and indentation when a change in the buffer occurs. Changing buffer state only causes that change to be recorded for later use. In fact, there are only a three cases that trigger a call to parinfer-rust--execute (the function that will manage parens and indentation). The first is when a command is executed locally to that buffer. In your example, executing a command from a magit buffer should not cause parinfer-rust--execute to run. The next is when first starting parinfer-rust-mode and a user has parinfer-rust-check-before-enable is false. Then it will run parinfer-rust--execute under paren mode. Again, magit revert shouldn't trigger parinfer-rust--execute. Finally, parinfer-rust--execute can be called when a user has set parinfer-rust-check-before-enable to true, parinfer-rust-mode detects that running under paren mode will cause a change in the buffer, and has the user agrees to this change. Again, this can not be triggered by magit reverting the buffer state.

I am starting to play with deferring when the parinfer-rust-check-before-enable. This means that it will only give a user a prompt on the first change in the buffer. This should substantially lower the amount of requests a users is given when said user loads a bunch of buffers at once. However, you are correct that the user will receive a prompt from parinfer-rust-mode when reverting a buffer. Ultimately, I am fine with this being the default and only actively supported behavior; users receiving prompts for buffers they actively induce a change on and otherwise leaving the buffer in an unmodified state. I feel like this implementation reaches a pretty good balance of minimal change in code maintenance, minimizing prompts to users, keeping feature parity with other implementations, and giving the user the least surprise about how and when things will be modified.

andreyorst commented 4 years ago

well thought.

More advanced hook system would be nice addition, but I wish I could change the behaviour of when to run parinfer -- on the buffer open, or on the first change, as I'm not the fan of running something for the first time after I've already changed something. In other words, I like the prompt, it's just a bit annoying when dealing with background buffers. I'll think about it more

justinbarclay commented 4 years ago

Hmm, a simple change would be to have parinfer-rust-check-before-enable be a list of options immediate, defer, nil and that would empower users to tweak the prompt that best suites their style and allow defaults to be set per project in the .dir-locals.el file.

andreyorst commented 4 years ago

with deferring this can also be closed

andreyorst commented 4 years ago

Hm, this seem to work quite well, and allow me to find which buffers are in fact visible, focused, or in background:

(defun buffer-visibility (buffer)
  (cond ((eq buffer (window-buffer (selected-window))) 'focused)
        ((get-buffer-window buffer) 'visible)
        (t 'background)))

I think this can be hooked to something and parinfer can indeed be configured to run only for active buffers. What do you think?

andreyorst commented 4 years ago

after a bit of testing this seem to be a rather expensive check.