From: Fermin <fmfs@posteo.net>
To: Stefan Monnier <monnier@iro.umontreal.ca>
Cc: emacs-devel@gnu.org
Subject: Re: [PATCH] (CEDET development) Improve the bovinate output buffer
Date: Fri, 2 Apr 2021 19:19:04 +0200 [thread overview]
Message-ID: <eb97432c-4671-b6c2-31d3-e51a25bc5bb2@posteo.net> (raw)
In-Reply-To: <jwv35w94r4k.fsf-monnier+emacs@gnu.org>
[-- Attachment #1: Type: text/plain, Size: 6385 bytes --]
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 #2: bovinate.patch --]
[-- Type: text/x-patch, Size: 1857 bytes --]
diff --git a/lisp/cedet/semantic.el b/lisp/cedet/semantic.el
index fb443fa4a3..a017caa7ff 100644
--- a/lisp/cedet/semantic.el
+++ b/lisp/cedet/semantic.el
@@ -335,25 +335,28 @@ semantic-elapsed-time
Arguments START and END bound the time being calculated."
(float-time (time-subtract end start)))
-(defun bovinate (&optional clear)
+(define-derived-mode bovinate-mode lisp-data-mode "Bovinate"
+ "Bovinate buffer major-mode.")
+
+(cl-defun bovinate (&optional clear (display t))
"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."
+If CLEAR is non-nil, it will do a full reparse.
+If DISPLAY is nil, it doesn't show the parser 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-name))))
+ (message "Retrieving tags took %.2f seconds." time-elapse)
+ (with-current-buffer (get-buffer-create bovinate-buffer)
(erase-buffer)
- (insert (pp-to-string out))
- (goto-char (point-min)))))
+ (insert (pp-to-string tags))
+ (bovinate-mode)
+ (goto-char (point-min)))
+ (when display (pop-to-buffer bovinate-buffer))))
+
\f
;;; Functions of the parser plug-in API
;;
prev parent reply other threads:[~2021-04-02 17:19 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-01 19:58 [PATCH] (CEDET development) Improve the bovinate output buffer Fermin
2021-04-01 22:02 ` Stefan Monnier
2021-04-02 17:19 ` Fermin [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=eb97432c-4671-b6c2-31d3-e51a25bc5bb2@posteo.net \
--to=fmfs@posteo.net \
--cc=emacs-devel@gnu.org \
--cc=monnier@iro.umontreal.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.