From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Fermin Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] (CEDET development) Improve the bovinate output buffer Date: Fri, 2 Apr 2021 19:19:04 +0200 Message-ID: References: <4776a4d8-e8c7-b087-2b3a-f3fa04b9fe99@posteo.net> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------65BFCE31D6BBFE15D36AD32D" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="16354"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.1 Cc: emacs-devel@gnu.org To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Fri Apr 02 19:20:36 2021 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lSNTI-0004AU-Ar for ged-emacs-devel@m.gmane-mx.org; Fri, 02 Apr 2021 19:20:36 +0200 Original-Received: from localhost ([::1]:48474 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lSNTH-0005EY-8V for ged-emacs-devel@m.gmane-mx.org; Fri, 02 Apr 2021 13:20:35 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:40810) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lSNS0-0003wn-OF for emacs-devel@gnu.org; Fri, 02 Apr 2021 13:19:16 -0400 Original-Received: from mout01.posteo.de ([185.67.36.65]:47600) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lSNRw-00061N-3c for emacs-devel@gnu.org; Fri, 02 Apr 2021 13:19:16 -0400 Original-Received: from submission (posteo.de [89.146.220.130]) by mout01.posteo.de (Postfix) with ESMTPS id 609CF16005C for ; Fri, 2 Apr 2021 19:19:06 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.net; s=2017; t=1617383946; bh=MJjvaqGNhnzroZtK/sXf3FicCBRzBLDbO6ow3HZbUr4=; h=Subject:To:Cc:From:Date:From; b=g4Kg26hwzFA8i4yFvscVOKZ2DwSsM3vrZ26yyJTKhG8rRhJmaqA/yPh7FZpNcLdgO c9vljZg9Hilprb5wxL98VzxTEZBJ3BvT8sEoHvwIEU1rOCeDBdAGmuu/wpORv2yufs PDfJsQ3ImiVlsEe/Sppqj4aPREDuLzHuLOlNUTPa/51KegwUhXi+WkEDLFa+LVaAdH CVgut3X64QPDj7612ey9sGcf7jMKhLmF76cG+9SCO3OxeVhmbKjt6To87ClH/lyu5g CSK3+4WGZ0pp3OkaQI5IsJKwRIR4su4Tg088epgh6BuSvHnVui2xAfrqfJ/gcOUTQx 7D6OzooHMNlHw== Original-Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4FBmxP1Bnlz9rxM; Fri, 2 Apr 2021 19:19:04 +0200 (CEST) In-Reply-To: Content-Language: en-US Received-SPF: pass client-ip=185.67.36.65; envelope-from=fmfs@posteo.net; helo=mout01.posteo.de X-Spam_score_int: -43 X-Spam_score: -4.4 X-Spam_bar: ---- X-Spam_report: (-4.4 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, NICE_REPLY_A=-0.001, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:267306 Archived-At: This is a multi-part message in MIME format. --------------65BFCE31D6BBFE15D36AD32D Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit 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 >> 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 >> 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 > --------------65BFCE31D6BBFE15D36AD32D Content-Type: text/x-patch; charset=UTF-8; name="bovinate.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="bovinate.patch" 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)))) + ;;; Functions of the parser plug-in API ;; --------------65BFCE31D6BBFE15D36AD32D--