emacsorphanage / helm-gtags

GNU GLOBAL helm interface
204 stars 31 forks source link

Parse file position revise #107

Closed Mikiya closed 9 years ago

Mikiya commented 9 years ago

Hello,

I updated the code and corrected some of them according to your advise. In addition, I added functions to handle the situation when the current point is not on defun, such as class, struct and macros.

The approach done in this patch is to run "global -f" when one of following conditions is met:

I suggest this approach based on an assumption that tag search done by global is faster than "global -f". If this is not the case, simply running "global -f" would be a good solution, instead of this approach.

Kind regards!

syohex commented 9 years ago

Is it necessary such complicated thing ?

Is it enough to following patch ?

diff --git a/helm-gtags.el b/helm-gtags.el
index ffdad1d..951f89f 100644
--- a/helm-gtags.el
+++ b/helm-gtags.el
@@ -1063,6 +1063,14 @@ Jump to reference point if curosr is on its definition"
                  this-file)))
     (setq helm-gtags--parsed-file (expand-file-name file))))

+(defun helm-gtags--function-beginning-line ()
+  (save-excursion
+    (beginning-of-defun)
+    (when (search-forward "(" nil t)
+      (goto-char (match-beginning 0))
+      (skip-chars-backward " \t\r\n")
+      (line-number-at-pos))))
+
 ;;;###autoload
 (defun helm-gtags-parse-file ()
   "Parse current file with gnu global. This is similar to `imenu'.
@@ -1077,9 +1085,9 @@ You can jump definitions of functions, symbols in this file."
                         (file-relative-name helm-gtags--parsed-file
                                             helm-gtags--tag-location))
                 helm-source-gtags-parse-file)
-  (let ((presel (when helm-gtags-preselect
-                  (format "^\\S-+\\s-+%d\\s-+"
-                          (line-number-at-pos)))))
+  (let* ((defun-line (helm-gtags--function-beginning-line))
+         (presel (when (and helm-gtags-preselect defun-line)
+                   (format "^\\S-+\\s-+%d\\s-+" defun-line))))
     (helm :sources '(helm-source-gtags-parse-file)
           :buffer helm-gtags--buffer :preselect presel)))
Mikiya commented 9 years ago

Hello,

I think complicated algorithm is required at the end of the day, because programming languages are complicated. One typical example I can show you now is a function returning a pointer to a function.

1: int
2: add(int x, int y)
3: {
4:   return x + y;
5: }
6: 
7: int
8: (*addfunc())(int x, int y) {
9:   return add;
10: }

As function name "addfunc" exists after a left bracket, the simple solution will return 7. Of course, global will return 8 as its line number.

Yet another scenario is a comment. I think this type of code cannot be handled easily.

int
/* this is a (bogus) comment */
bogusfunc()
{
  puts("bogus");
}

Parsing a source code is a difficult theme. In order to parse variety of programming languages, we need to invent a parser like global. Why not we use global then?

So, actually, running "global -f" is sufficient for this purpose. This is done in function "helm-gtags--previous-tag-from-here" in my patch. This function searches a previous line retrieved from "global -f". The retrieved line number must be identical to what is retrieved by the second execution of "global -f".

Function "helm-gtags--tag-line-number-in-bound" is non-essential. It is a performance hack based on an assumption that tag search is faster than "global -f". If you would like to keep things as simple as possible, I suggest to remove this performance hack, they are "helm-gtags--tag-line-number-in-bound", "helm-gtags--which-function-only-func-name" and "helm-gtags--tag-included-in-bound".

Kind regards!

syohex commented 9 years ago

How about this ?

diff --git a/helm-gtags.el b/helm-gtags.el
index ffdad1d..a164bbc 100644
--- a/helm-gtags.el
+++ b/helm-gtags.el
@@ -1063,6 +1063,26 @@ Jump to reference point if curosr is on its definition"
                  this-file)))
     (setq helm-gtags--parsed-file (expand-file-name file))))

+(defun helm-gtags--function-beginning-line ()
+  (let ((defun-bound (bounds-of-thing-at-point 'defun)))
+    (if (not defun-bound)
+        0
+      (let ((defun-begin-line (line-number-at-pos (car defun-bound)))
+            (filename (helm-gtags--real-file-name)))
+        (with-temp-buffer
+          (unless (zerop (process-file "global" nil t nil "-f" filename))
+            (error "Failed: global -f"))
+          (goto-char (point-min))
+          (let (start-line)
+            (while (and (not start-line)
+                        (re-search-forward "^\\S-+\\s-+\\([1-9][0-9]*\\)" nil t))
+              (let ((line (string-to-number (match-string-no-properties 1))))
+                (when (>= line defun-begin-line)
+                  (setq start-line line))))
+            (if (not start-line)
+                0
+              start-line)))))))
+
 ;;;###autoload
 (defun helm-gtags-parse-file ()
   "Parse current file with gnu global. This is similar to `imenu'.
@@ -1077,9 +1097,9 @@ You can jump definitions of functions, symbols in this file."
                         (file-relative-name helm-gtags--parsed-file
                                             helm-gtags--tag-location))
                 helm-source-gtags-parse-file)
-  (let ((presel (when helm-gtags-preselect
-                  (format "^\\S-+\\s-+%d\\s-+"
-                          (line-number-at-pos)))))
+  (let* ((defun-line (helm-gtags--function-beginning-line))
+         (presel (when (and helm-gtags-preselect defun-line)
+                   (format "^\\S-+\\s-+%d\\s-+" defun-line))))
     (helm :sources '(helm-source-gtags-parse-file)
           :buffer helm-gtags--buffer :preselect presel)))
Mikiya commented 9 years ago

Thank you very much for your taking care of my opinion! Calling "global -f" makes thing done well :)

Yet another my suggestion is handling of (bounds-of-thing-at-point 'defun). It cannot handle macros, while they will be included in "global -f" output.

1: extern "C" { 2: #define A_MACRO(a) a++ 3: }

Surprisingly, global -f shows a macro at line 2, but (bounds-of-thing-at-point 'defun) returns a point for line 1 (extern "C"). I think comparing with (line-number-at-pos) is sufficient for any cases, because current position of cursor (point) is the very significant information to construct preselect. In other words, comparing output of "global -f" and current line number is sufficient IMHO.

Kind regards!