FelipeLema / emacs-counsel-gtags

GNU Global with ivy completion
22 stars 5 forks source link

Critical issue with counsel-gtags-find-definition and other commands #15

Closed Ergus closed 3 years ago

Ergus commented 3 years ago

Hi:

Recently when I try to use some counsel-gtags commands I get this errors: (wrong-number-of-arguments (1 . 2) 0)

Lookign at the stack I get this:

Debugger entered--Lisp error: (wrong-number-of-arguments (1 . 2) 0)
  counsel--elisp-to-pcre()
  counsel-gtags--build-command-to-collect-candidates(#("ClusterManager" 0 14 (face font-lock-type-face fontified t)) ("--result=ctags"))
  counsel-gtags--async-tag-query-process(#("ClusterManager" 0 14 (face font-lock-type-face fontified t)))
  counsel-gtags--async-tag-query(#("ClusterManager" 0 14 (face font-lock-type-face fontified t)))
  ivy--dynamic-collection-cands(#("ClusterManager" 0 14 (face font-lock-type-face fontified t)))
  ivy--reset-state(#s(ivy-state :prompt "Find Definition: " :collection counsel-gtags--async-tag-query :predicate nil :require-match nil :initial-input #("ClusterManager" 0 14 (face font-lock-type-face fontified t)) :history nil :preselect nil :keymap nil :update-fn nil :sort nil :frame #<frame F1 0x5568d334d908> :window #<window 1 on ClusterManager.cpp> :buffer #<buffer ClusterManager.cpp> :text nil :action (1 ("o" identity "default") ("i" ivy--action-insert "insert") ("w" ivy--action-copy "copy")) :unwind #f(compiled-function () #<bytecode 0x1e0c7d770d11>) :re-builder ivy--regex-plus :matcher nil :dynamic-collection t :display-transformer-fn nil :directory "/home/ergo/PhD/nanos6/nanos6_cluster/src/cluster/" :caller counsel-gtags--read-tag :current nil :def nil :ignore t :multi-action nil :extra-props nil))
  ivy-read("Find Definition: " counsel-gtags--async-tag-query :initial-input #("ClusterManager" 0 14 (face font-lock-type-face fontified t)) :unwind #f(compiled-function () #<bytecode 0x1e0c7d770d11>) :dynamic-collection t :caller counsel-gtags--read-tag)
  apply(ivy-read ("Find Definition: " counsel-gtags--async-tag-query :initial-input #("ClusterManager" 0 14 (face font-lock-type-face fontified t)) :unwind #f(compiled-function () #<bytecode 0x1e0c7d770d11>) :dynamic-collection t :caller counsel-gtags--read-tag))
  counsel-gtags--read-tag(definition)
  byte-code("\300\301!C\207" [counsel-gtags--read-tag definition] 2)
  call-interactively(counsel-gtags-find-definition record nil)
  command-execute(counsel-gtags-find-definition record)
  counsel-M-x-action("counsel-gtags-find-definition")
  ivy-call()
  ivy-read("M-x " ("debug-on-entry" "toggle-debug-on-error" "counsel-gtags-find-definition" "package-list-packages" "gpm-mouse-mode" "magit-status" "sudo-edit" "magit-push-to-remote" "man" "undo" "mu4e" "tabify" "copy-file" "magit-pull" "magit-log-all" "cd" "5x5" "amx" "arp" "dbx" "dig" "erc" "ert" "eww" "ftp" "gdb" "irc" "jdb" "lsp" "mpc" "pdb" "pwd" "rsh" "sdb" "xdb" "bang" "calc" "diff" "dirs" "ffap" "gnus" "grep" "help" "ielm" "info" "lice" "life" "mail" "mpuz" "ping" ...) :predicate #f(compiled-function (x) #<bytecode 0xa5d869007dcd8f>) :require-match t :history counsel-M-x-history :action counsel-M-x-action :keymap (keymap (67108908 . counsel--info-lookup-symbol) (67108910 . counsel-find-symbol)) :initial-input nil :caller counsel-M-x)
  counsel-M-x()
  funcall-interactively(counsel-M-x)
  call-interactively(counsel-M-x nil nil)
  command-execute(counsel-M-x)
FelipeLema commented 3 years ago

hey there, thanks for reporting this issue

I cannot reproduce this, though. Can you provide a minimum working example? Check the tests in this package for inspiration.

Ergus commented 3 years ago

Hi Felipe:

Probably you are using an older version of Ivy?

The function counsel--elisp-to-pcrerequires a first mandatory argument REGEX. That's the reason of the error. The call:

(counsel--elisp-to-pcre)

FelipeLema commented 3 years ago

Hey, Ergus

I agree with you that it takes the REGEX argument, but that is a consquence of the real problem. You see, thread-first is passing the return value of (ivy--regex query), which seems to be nil in this case.

If I run (counsel-gtags--build-command-to-collect-candidates "ClusterManager" '("--result=ctags")) it works all right (your tracelog shows the fontified text... text with color and formatting). I don't know if the fontification of a string affects ivy--regex-query, but I cannot try since I don't know how to translate the lisp object in the tracelog into something that can be run regardless.

Ergus commented 3 years ago

Hey, Ergus

I agree with you that it takes the REGEX argument, but that is a consquence of the real problem. You see, thread-first is passing the return value of (ivy--regex query), which seems to be nil in this case.

To be save thing-at-point should be called with the extra third parameter NO-PROPERTIES as non-nil OR use the function ivy-thing-at-point instead. The second alternative is prefered because it is smarter.

If I run (counsel-gtags--build-command-to-collect-candidates "ClusterManager" '("--result=ctags")) it works all right (your tracelog shows the fontified text... text with color and formatting). I don't know if the fontification of a string affects ivy--regex-query,

It seems so indeed.

but I cannot try since I don't know how to translate the lisp object in the tracelog into something that can be run regardless.

(counsel-gtags--build-command-to-collect-candidates #("ClusterManager" 0 14 (fontified t face font-lock-constant-face)) '("--result=ctags"))

FelipeLema commented 3 years ago

(counsel-gtags--build-command-to-collect-candidates #("ClusterManager" 0 14 (fontified t face font-lock-constant-face)) '("--result=ctags"))

I just ran this and it works without an error. Do you see an error on your side after running this?

I'm using Emacs 27.1 and ivy 0.13.0

Thanks for the time you've invested on this

Ergus commented 3 years ago

When I run that code I get the same error than in the backtrace.

image

I have emacs-28.1 and ivy-20200826.955. I keep the suggestion of using ivy-thing-at-point.

BTW: Why do you pipe to grep there instead of adding the -g option? Should be more efficient and simplify the code notably right??.

Ergus commented 3 years ago

After a bit more of debugging I got this backtrace:

Debugger entered--returning value: nil
  eval-expression-print-format(#("ClusterManager" 0 14 (face font-lock-constant-face fontified t)))
  #f(compiled-function (exp &optional insert-value no-truncate char-print-limit) "Evaluate EXP and print value in the echo area.\nWhen called interactively, read an Emacs Lisp expression and\nevaluate it.  Value is also consed on to front of the variable\n`values'.  Optional argument INSERT-VALUE non-nil (interactively,\nwith a non `-' prefix argument) means insert the result into the\ncurrent buffer instead of printing it in the echo area.\n\nNormally, this function truncates long output according to the\nvalue of the variables `eval-expression-print-length' and\n`eval-expression-print-level'.  When NO-TRUNCATE is\nnon-nil (interactively, with a prefix argument of zero), however,\nthere is no such truncation.\n\nIf the resulting value is an integer, and CHAR-PRINT-LIMIT is\nnon-nil (interactively, unless given a non-zero prefix argument)\nit will be printed in several additional formats (octal,\nhexadecimal, and character).  The character format is used only\nif the value is below CHAR-PRINT-LIMIT (interactively, if the\nprefix argument is -1 or the value doesn't exceed\n`eval-expression-print-maximum-character').\n\nRuns the hook `eval-expression-minibuffer-setup-hook' on entering the\nminibuffer.\n\nIf `eval-expression-debug-on-error' is non-nil, which is the default,\nthis command arranges for all errors to enter the debugger." (interactive #f(compiled-function () #<bytecode 0xbe081f1dedb4bbf>)) #<bytecode 0x95bdd7256e9d1ab>)((ivy--regex #("ClusterManager" 0 14 (fontified t face font-lock-constant-face))) nil nil 127)
  apply(#f(compiled-function (exp &optional insert-value no-truncate char-print-limit) "Evaluate EXP and print value in the echo area.\nWhen called interactively, read an Emacs Lisp expression and\nevaluate it.  Value is also consed on to front of the variable\n`values'.  Optional argument INSERT-VALUE non-nil (interactively,\nwith a non `-' prefix argument) means insert the result into the\ncurrent buffer instead of printing it in the echo area.\n\nNormally, this function truncates long output according to the\nvalue of the variables `eval-expression-print-length' and\n`eval-expression-print-level'.  When NO-TRUNCATE is\nnon-nil (interactively, with a prefix argument of zero), however,\nthere is no such truncation.\n\nIf the resulting value is an integer, and CHAR-PRINT-LIMIT is\nnon-nil (interactively, unless given a non-zero prefix argument)\nit will be printed in several additional formats (octal,\nhexadecimal, and character).  The character format is used only\nif the value is below CHAR-PRINT-LIMIT (interactively, if the\nprefix argument is -1 or the value doesn't exceed\n`eval-expression-print-maximum-character').\n\nRuns the hook `eval-expression-minibuffer-setup-hook' on entering the\nminibuffer.\n\nIf `eval-expression-debug-on-error' is non-nil, which is the default,\nthis command arranges for all errors to enter the debugger." (interactive #f(compiled-function () #<bytecode 0xbe081f1dedb4bbf>)) #<bytecode 0x95bdd7256e9d1ab>) ((ivy--regex #("ClusterManager" 0 14 (fontified t face font-lock-constant-face))) nil nil 127))
  eval-expression((ivy--regex #("ClusterManager" 0 14 (fontified t face font-lock-constant-face))) nil nil 127)
  funcall-interactively(eval-expression (ivy--regex #("ClusterManager" 0 14 (fontified t face font-lock-constant-face))) nil nil 127)
  call-interactively(eval-expression nil nil)
  command-execute(eval-expression)

The point seems to be that ivy--regex gets confused by the # and tries to apply it as a function. In any case the output for this will have the face information which is expected to be wrong for grep. The propperties removal needs to be done one way or the other.

FelipeLema commented 3 years ago

I guess we can make a copy of query without fontication and that should solve the issue.

I just uploaded an unit test for this issue, but it's passing. I guess it's an Emacs 28 thing? I'll dig for a travis conf for testing emacs nightly.

FelipeLema commented 3 years ago

I updated travis to use Emacs 28.0.50 (snapshot) and I still see the unit test for this issue as passing.

Setup using straight.el is using "latest" Ivy (whatever MELPA says at the moment the test starts running). So I'm kinda lost here on how to reproduce this problem.

@Ergus can you confirm that using (substring-no-properties query) works?

Ergus commented 3 years ago

Hi Felipe:

BTW: I made a lot of changes in my personal fork of the project. Basically to simplify and reduce some uneeded code. Probably you will be not interested in all the changes because I did changes like remove the lists and use concat because it simplifies a lot of problems for me (That's why I don't do a pull request unless you want), but maybe you could be interested in some of them like the maps, or some functions I substituted with simpler alternatives.

(Just compare with the final commit with your branch, because as this is a personal fork the intermediate commits may be broken)

Best, Ergus

Ergus commented 3 years ago

@Ergus can you confirm that using (substring-no-properties query) works?

Ahh yes with that it works I tried before.

FelipeLema commented 3 years ago

you can go ahead and do a Pull Request. I suggest you do it in a separate branch (something like "solves-15-and-other-cool-stuff") so we can do a code review before merging it to main branch

Ergus commented 3 years ago

you can go ahead and do a Pull Request.

Done.

I suggest you do it in a separate branch (something like "solves-15-and-other-cool-stuff") so we can do a code review before merging it to main branch

It would be nice if you could add new tests to the new functions because I don't have more time. ATM everything is working fine because I am using that. But could be nice to have some automatic tests.

Maybe if you could add some ecukes tests to tests the maps could be interesting.