emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] (CEDET development) Improve the bovinate output buffer


From: Fermin
Subject: Re: [PATCH] (CEDET development) Improve the bovinate output buffer
Date: Fri, 2 Apr 2021 19:19:04 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.1

Thanks for the feedback!

AFAICT this is better served by `lisp-data-mode` than `emacs-lisp-mode`, no?
Also, try to make the first line of docstrings be self-contained.
Maybe you could just use `lisp-data-mode` without bothering to
define a new major mode for it.

I couldn't find the lisp-data-mod because I'm developing in the Emacs 27.1 , I did have to change my configuration
so now I can use the build from master.

The idea of the major mode is that I want to add some interactive behavior, e.g improve the tag navigation with buttons
, so a new major mode can help in that task.

I did change the function from defun to cl-defun, so I can set the default value of display to true, the idea of non displaying the
source code file is for testing mainly.

This would better be folded into the previous patch.
Done, I think I address most of the issues, sorry for the inconvenience, I'm not use to work with mailing list development.

I attach a patch with the changes.

Regards


On 02/04/2021 00:02, Stefan Monnier wrote:
 From f8c7ee1791ec6eee1993a9e5449112579e193415 Mon Sep 17 00:00:00 2001
From: Fermin <fmfs@posteo.net>
Date: Thu, 1 Apr 2021 21:31:56 +0200
Subject: [PATCH] Add bovinate-mode as a major mode and improve the bovinate
  command

---
Could you expand this commit message a bit and clarify on the first line
that this regards some part of semantic?  E.g.:

     Subject: [PATCH] semantic.el (bovinate): Prettify output

     * lisp/cedet/semantic.el (bovinate-mode): New major mode.
     (bovinate): ???.

where the `???` should say what kind of improvement is brought by the
patch (like "Give a more meaningful name to the buffer"?).

+(define-derived-mode bovinate-mode emacs-lisp-mode "Bovinate"
+  "Bovinate buffer major-mode, giving that the list is generated with
+`pp-to-string', the syntax is very similar to a list of Emacs Lisp.
+This major mode helps with the syntax highlight of some of the symbols.")
AFAICT this is better served by `lisp-data-mode` than `emacs-lisp-mode`, no?
Also, try to make the first line of docstrings be self-contained.
Maybe you could just use `lisp-data-mode` without bothering to
define a new major mode for it.

  (defun bovinate (&optional clear)
    "Parse the current buffer.  Show output in a temp buffer.
  Optional argument CLEAR will clear the cache before parsing.
  If CLEAR is negative, it will do a full reparse, and also display
  the output buffer."
    (interactive "P")
-  (if clear (semantic-clear-toplevel-cache))
-  (if (eq clear '-) (setq clear -1))
+  (when clear (semantic-clear-toplevel-cache))
    (let* ((start (current-time))
-        (out (semantic-fetch-tags)))
-    (message "Retrieving tags took %.2f seconds."
-            (semantic-elapsed-time start nil))
-    (when (or (null clear) (not (listp clear))
-             (and (numberp clear) (< 0 clear)))
-      (pop-to-buffer "*Parser Output*")
-      (require 'pp)
+        (tags (semantic-fetch-tags))
+         (time-elapse (semantic-elapsed-time start nil))
+         (bovinate-buffer
+          (format "*Parser Output from %s buffer*" (buffer-name))))
+    (message "Retrieving tags took %.2f seconds." time-elapse)
+    (unless (get-buffer bovinate-buffer)
+      (setq bovinate-buffer (get-buffer-create bovinate-buffer)))
+    (with-current-buffer bovinate-buffer
I think you can simplify the last three lines to

        (with-current-buffer (get-buffer-create bovinate-buffer)

        (erase-buffer)
-      (insert (pp-to-string out))
+      (insert (pp-to-string tags))
+      (bovinate-mode)
        (goto-char (point-min)))))
I don't see where the new code implements the "negative CLEAR" behavior.
Also I don't see in the new code where/when the buffer is displayed.
[ Read on before replying ;-)  ]

 From 1c1673283186780e6db007f79d4f8aa864660f79 Mon Sep 17 00:00:00 2001
From: Fermin <fmfs@posteo.net>
Date: Thu, 1 Apr 2021 21:53:16 +0200
Subject: [PATCH] Add display as a optional parameter for displaying the buffer

---
Similar comment abut the commit message.

  (define-derived-mode bovinate-mode emacs-lisp-mode "Bovinate"
    "Bovinate buffer major-mode, giving that the list is generated with
-`pp-to-string', the syntax is very similar to a list of Emacs Lisp.
+`pp-to-string', the syntax is very similar to a list in Emacs Lisp.
  This major mode helps with the syntax highlight of some of the symbols.")
This would better be folded into the previous patch.

-(defun bovinate (&optional clear)
-  "Parse the current buffer.  Show output in a temp buffer.
+(defun bovinate (&optional clear display)
+  "Parse the current buffer.  Show output in a `bovinate-mode' buffer.
  Optional argument CLEAR will clear the cache before parsing.
-If CLEAR is negative, it will do a full reparse, and also display
-the output buffer."
+If non-nil DISPLAY will display the buffer `pop-to-buffer'."
    (interactive "P")
This means that when used interactively DISPLAY will always be nil and
hence the buffer will never be "popped".  This contradicts the
first line's "Show output in a `bovinate-mode' buffer" and I also wonder
what is the benefit (is it often useful/necessary to do
`M-x bovinate` without wanting to see the resulting tags)?

    (when clear (semantic-clear-toplevel-cache))
    (let* ((start (current-time))
         (tags (semantic-fetch-tags))
           (time-elapse (semantic-elapsed-time start nil))
           (bovinate-buffer
-          (format "*Parser Output from %s buffer*" (buffer-name))))
+          (format "*Parser Output from %s*" (buffer-name))))
This should also be folded into the previous patch.

      (message "Retrieving tags took %.2f seconds." time-elapse)
      (unless (get-buffer bovinate-buffer)
        (setq bovinate-buffer (get-buffer-create bovinate-buffer)))
@@ -359,7 +358,8 @@ the output buffer."
        (erase-buffer)
        (insert (pp-to-string tags))
        (bovinate-mode)
-      (goto-char (point-min)))))
+      (goto-char (point-min)))
+    (when display (pop-to-buffer bovinate-buffer))))
Hmm... overall, I think the two patches should be squashed together.


         Stefan

Attachment: bovinate.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]