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

Can't delete function call using dsf. #41

Closed
tmalsburg opened this issue Oct 26, 2014 · 27 comments
Closed

Can't delete function call using dsf. #41

tmalsburg opened this issue Oct 26, 2014 · 27 comments

Comments

@tmalsburg
Copy link

Since it's possible to wrap a text object in a function call with, e.g., yssf, I would expect dsf to delete that function call. Seems like a very useful feature.

@tmalsburg
Copy link
Author

Similarly, it's not possible to change a function call to something else using csf.

@HalimNaim
Copy link

I have pushed a first attempt at removing a function. It will not change it though. The problem with deleting functions, is that it will try to get an outer and inner overlay from evil using f as the char argument, for which evil has not text-objects. I added a test case in the cond for ?f And implemented a function that will delete it. It will cause problems if called outside the parens of the function call however. I don't know if I should limit it to a single line or not. commit

@HalimNaim
Copy link

I've re-written evil-surround-delete-function, so it uses evil procedures. I have tested and it works pretty well. I have commited it to my test branch. Please review and test it. And let me know if you find anything wrong with it.

@tmalsburg
Copy link
Author

Thanks for working on this! I'm testing it right now. It works in Python and R scripts but for some reason not in the scratch buffer. There it is doing weird things, deleting wrong characters. As you say, it only works correctly when the cursor is in the parenthesis. Perhaps, you could change it to do nothing at all when the cursor is outside? That would be much better than the erratic behaviour that we get with the current code when the point is outside.

Here is an interesting case:

funB(funA((arg))

When you place the cursor on arg and enter dsf it deletes funA. It would be extremely usefule if it was also possible to delete funB instead of funA. (If I delete a function, it's most of the time the most outer function.) Would it be possible to reuse (or rather abuse) Vim's inner and outer semantics for that? I'm thinking of using dsif for deleting funA and dsaf for deleting funB.

@HalimNaim
Copy link

You mean emacs' scratch buffer? Thats wierd. Do you have a test case? It should do nothing when it is outside. I'm relying on '[ (' Which doesn't move unless it's inside the function. In the test case you post, you have unbalanced parens. I don't know how to handle that case. If the parens where balanced, you could delete funB, by placing point on any of 'funA('.

@tmalsburg
Copy link
Author

Sorry, there was a typo in the test case. What I meant was of course this:

funB(funA(arg))

I can't reproduce the erratic behaviour in the scratch buffer, so forget that. However, when I place the cursor on "u" in the following example

funA(arg)

and enter dsf, it deletes "fun" and the final ")" and leaves me with:

A(arg

This happens in the scratch buffer and in other buffers as well.

@HalimNaim
Copy link

Are you sure you are using the version in my test branch? I'm not getting the behavior you describe. If I place the point anywhere in funA( I get funB(arg) And if I have funA(arg) placing point in arg) gets me just arg

@tmalsburg
Copy link
Author

I'm using ac3017c from the test branch. I think I found the problem: in line 171, you assume that evil-previous-open-paren returns nil when there is no opening parenthesis but that's not the case. At least in my setup (current development Emacs, current Evil from MELPA), I get -1 if there is no opening parenthesis. So the body of the when clause is always executed, irrespective of whether there is an opening parenthesis or not.

@HalimNaim
Copy link

Thats strange.

(evil-define-motion evil-previous-open-paren (count)
   "Go to [count] previous unmatched '('."
   :type exclusive
   (let ((range (save-excursion
                        (backward-char)
                        (evil-paren-range count nil nil nil ?\( ?\)))))
       (when range
            (goto-char (evil-range-beginning range)))))

Here is the code of the function. If range is nil, when should return nil. And that should be the return value of the procedure in that case. The debugger confirms that. I have Emacs 24.3.1 M-x evil-version returns 1.0-dev.

@tmalsburg
Copy link
Author

On my system, I have this definition:

(evil-define-motion evil-previous-open-paren (count)
  "Go to [count] previous unmatched '('."
  :type exclusive
  (evil-up-paren ?( ?) (- (or count 1))))

This is also the version in the current development version of Evil. Git blame says that this function was changed in January by Frank Fischer. See here.

@HalimNaim
Copy link

Oops. I have pulled evil master. And updated the function to test for -1 first, then thought it would be better to test for 0. So I made another commit to fix that. Should work now.

@tmalsburg
Copy link
Author

Works, nice. The question is whether we can rely on the return value of evil-previous-open-paren. The documentation doesn't specify return values and future changes may therefore break your code. A safer way to test for an opening parenthesis would be to test:

(= (point) (save-excursion (evil-previous-open-paren) (point)))

@HalimNaim
Copy link

Good Idea. Done!. Just changed save-excursion with progn becuase I need point to be at the paren.

@tmalsburg
Copy link
Author

Ah, right.

@TheBB
Copy link
Member

TheBB commented Apr 21, 2016

I would like to work on this problem. It plagues more than just the function surround, also things like #69. Is there no word from @timcharper on how he would like this to look?

@HalimNaim
Copy link

well, I stopped using evil since then for vanilla emacs. My implementation worked just fine for me. It deleted the surrounding function, but I didn't get around at implementing changing the surrounding function. IIRC. Tim proposed this addition to vim surround and it wasn't accepted so he didn't include it here for compatibility with the original. I may be wrong though.

@timcharper
Copy link
Collaborator

Yeah, this project targets compatibility with vim.surround... for... reasons. The plugin, right now (for the most part...), doesn't have any special considerations for different major modes. I'd like to keep it that way. There's no standard Emacs way to define an overlay for a function. It just doesn't exist. This feature would require a special function for every major mode people use.

If you come up with a snippet that people can paste in to their .emacs, and is reasonably extensible, I'll totally link to it from the README!

@TheBB
Copy link
Member

TheBB commented Apr 22, 2016

This feature would require a special function for every major mode people use.

Adding a surrounding function isn't specialized by major mode either, though. The feature I was looking for was for dsf to remove precisely what v<...>sf adds.

@timcharper
Copy link
Collaborator

Those keybindings don't seem to work for me. And, I'm not sure what you mean by "adding a surrounding function isn't specialized by major mode". It's syntax specific. In emacs-lisp-mode, surrounding with a function is done with (defun fn-name (...)). In ruby, it's def fn_name; ...; end. In Scala, it's def fnName = {}.

@TheBB
Copy link
Member

TheBB commented Apr 28, 2016

Really? That's not what I'm seeing. Let me use non-visual state so the bindings are the same. For example with cursor on the word word the key sequence ysiwfa<RET> produces a(word) in Emacs Lisp mode, not (a word) which is what I would have expected if it were syntax aware.

Are we talking about the same thing?

@timcharper
Copy link
Collaborator

Okay; I've never used the key sequence. You're right, it does work, and it doesn't the right thing lisps (it assumes that all functions in all modes are called with identifier(params), and never (identifier params), corroborating my point that language specific support would be required.

@timcharper
Copy link
Collaborator

timcharper commented May 2, 2016

Is this supported in vim-surround, though? It sounded like @tpope shied away from it.

@timcharper
Copy link
Collaborator

tpope/vim-surround#157

@timcharper
Copy link
Collaborator

tpope/vim-surround#118

@braham-snyder
Copy link

for those seeking a quick fix, https://github.com/cute-jumper/embrace.el combined with evil-embrace (linked there--hooks into evil-surround with one line) allows (non-language-aware) dsf and csf

@tmalsburg
Copy link
Author

Awesome, thanks for sharing, @braham-snyder.

@ninrod
Copy link
Member

ninrod commented Jan 16, 2017

With the existence of evil-embrace, I think we can close this now.

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

No branches or pull requests

6 participants