From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: ~Make emacs friendlier: package documentation [POC CODE INCLUDED] Date: Wed, 21 Oct 2020 18:31:53 -0400 Message-ID: References: <20201015190929.gdvx7j2yukcdcoaw@E15-2016.optimum.net> <83pn5jwav0.fsf@gnu.org> <20201015194132.jdn3e2v62vfvh7ju@E15-2016.optimum.net> <83imbawu6y.fsf@gnu.org> <20201016073432.4bmahi4jna2xxayl@E15-2016.optimum.net> <20201018144345.wgrbsivvzuuohbj5@E15-2016.optimum.net> <20201018162030.h7s4tqch3sprssxu@E15-2016.optimum.net> <87a6wjnu8f.fsf@gmail.com> <20201019025507.5s5yq2wnny7dttqm@E15-2016.optimum.net> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="38240"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Cc: emacs-devel@gnu.org To: Boruch Baum Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Thu Oct 22 00:33:21 2020 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 1kVMfZ-0009qk-35 for ged-emacs-devel@m.gmane-mx.org; Thu, 22 Oct 2020 00:33:21 +0200 Original-Received: from localhost ([::1]:44018 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kVMfY-0006Rv-4b for ged-emacs-devel@m.gmane-mx.org; Wed, 21 Oct 2020 18:33:20 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:33676) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kVMeJ-0005uO-PX for emacs-devel@gnu.org; Wed, 21 Oct 2020 18:32:04 -0400 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:25622) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kVMeG-0001sP-Jt for emacs-devel@gnu.org; Wed, 21 Oct 2020 18:32:02 -0400 Original-Received: from pmg3.iro.umontreal.ca (localhost [127.0.0.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id DF2384416AE; Wed, 21 Oct 2020 18:31:56 -0400 (EDT) Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id BEC374416AA; Wed, 21 Oct 2020 18:31:54 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1603319514; bh=K4oj9oM34EhCbBgnZIJjEbm11E43lMb70tIYiCF9is8=; h=From:To:Cc:Subject:References:Date:In-Reply-To:From; b=fNQBpjCBx6q9sbTfQe+xMmLraaV5JzKodsoErr/l+IdIpEXxIFaHhGjaA+FeP28jc Ehdbqw1ZbRczaIpRY4j3It3yprEeWUBFtJTzK/l1YINppkD/590Q+fK0yPXzykfpbb RrIM8OwKnPZ67sfRz01NW8aFSpl3mcllhSkFnqtrchKQIAXULs/Tq7xC4IqCiP0Yhv sdTqS0bL1XBUwfNB7nzsFa01f2PkjXiD203lFwfvVAg6N7yc9NWSEQRZ917/HlBJLM /pSB/KgteYw/PdtP9Sak5d5xqcbnrAIplbc6pLNFB1eRyeyzwxxPfj2AvoSk5jHuui O0YxtwZo3+2lA== Original-Received: from alfajor (unknown [157.52.9.240]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 899E5120257; Wed, 21 Oct 2020 18:31:54 -0400 (EDT) In-Reply-To: <20201019025507.5s5yq2wnny7dttqm@E15-2016.optimum.net> (Boruch Baum's message of "Sun, 18 Oct 2020 22:55:07 -0400") Received-SPF: pass client-ip=132.204.25.50; envelope-from=monnier@iro.umontreal.ca; helo=mailscanner.iro.umontreal.ca X-detected-operating-system: by eggs.gnu.org: First seen = 2020/10/21 16:32:28 X-ACL-Warn: Detected OS = Linux 2.2.x-3.x [generic] X-Spam_score_int: -42 X-Spam_score: -4.3 X-Spam_bar: ---- X-Spam_report: (-4.3 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_MED=-2.3, 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:258259 Archived-At: Hi, here are my comments written on the fly as I read your code. While I'm here: have you looked into what it would take to add the magic link to the `C-h P` page? Stefan > ;;; org-el-file.el --- Make org documetation from elisp source file -*- lexical-binding: t -*- [ Of course, this file would need the usual copyright/license blurb, here, as well as the `;;; Commentary:` and `;;; Code:` sections so it can be nicely viewed with itself. ] > (defun org-el-file (file) > "Create documentation in org-mode format from FILE. > FILE is an elisp source file formatted according to the emacs ^^^^^ I think our illustrious software's name deserves to be capitalized ;-) > style. Result is an org mode buffer containing the file's ^^^ By convention, we put two spaces between sentences (this is used by sentence navigation and filling according to `sentence-end-double-space`). > doumentary comments and docstrings." > (interactive "f") I think this should use an interactive spec similar to that of `find-library`. > (switch-to-buffer (get-buffer-create (concat (file-name-nondirectory file) ".org"))) Please never ever use `switch-to-buffer` in your ELisp code, since its meaning is unclear (OT1H it says "change content of selected window" and OTOH it says "display this buffer", making it unclear what should happen when this buffer can't be displayed in the selected window). E.g. Use `display-buffer`, `pop-to-buffer`, `pop-to-buffer-same-window`, ... The buffer name you chose looks like a file-buffer's name, whereas this is a "special buffer" not linked to any file, for which we traditionally use names surrounded by stars. Also, I don't think ".org" is needed there. One more thing, using just (file-name-nondirectory file) isn't good enough for files like `lisp/cedet/semantic/bovine/debug.el` (since that conflicts with `lisp/emacs-lisp/debug.el`; the package's name is `semantic/bovine/debug.el`). If you use my recommendation above for the interactive spec, then the user will have to write something like "semantic/bovine/debug" and you can reuse that name directly without having to strip anything from it. Of course, you could also opt to go in the direction of making your buffer into a "real" file-buffer (or even a real file) in which case you could use `create-file-buffer` (which will choose the right buffer name based on the usual rules such as uniquify). > (insert-file-contents file nil nil nil 'replace) > (goto-char (point-min)) > ;; Comment-out docstrings > (let (p0 p1 p2) > (while (setq p0 (re-search-forward "^(def" nil t)) > (when (not (re-search-forward "^ +\"" nil t)) > (error "badly formatted file, near %d" p0)) [ You can rewrite `when (not` to `unless`. ] [ You don't need `p0` here, you can use (match-beginning 0) instead. ] I don't think we should treat this as an error. Also, I think it's important to bound the search to the sexp that's started by the open-paren. Many sexps that start with `(def` don't have docstrings (e.g. all those (defvar FOO) which are just there to tell the byte-compiler that we're expecting those vars to be defined elsewhere), so we should be careful not to incorrectly match a docstring with some unrelated previous `(defvar`. > (setq p1 (match-beginning 0)) I recommend you use `let` for it here (so you never need to `setq` the variable, which saves you from having to think about which value of that variable you're getting when you refer to it). Same for `p2`: use `let` at the point where you can give it its real value rather than having a "dummy" let followed by a `setq` later. > (replace-match "") > (when (not (re-search-forward "\")?$" nil t)) > (error "badly formatted file, near %d" p0)) A " char can legitimately appear at the end of a line *within* a docstring . Better use `forward-sexp` to jump over the docstring while obeying the usual backslash escaping rules (but make sure you set the buffer in `emacs-lisp-mode`, first). > (goto-char p1) > (narrow-to-region p1 p2) ; because p2 moves with every replacement > (while (re-search-forward "^" nil t) > (replace-match ";;")) > (widen))) The better option, IMO is to do (goto-char p2) (while (> (point) p1) ... (forward-line -1) ...) > ;; Comment-out def* and adjust pre-existing comments > (dolist (repl '(("^;;; " ";;;; ") > ("^$" ";;") > ("^(def" ";;; (def"))) Traditionally the outline convention used in Elisp is that ";;; " is a top-level heading, ";;;; " is a subheading, ";;;;; " is a subsubheading (and ";;" is not a heading at all), whereas you seem to use a convention that's inverted in this respect, which will not play well with files which use ";;;; " and friends (which are often the better structured ones, IME). > (dolist > (repl '(("^;;;;" "**") > ("^;;; (def\\([^ ]+\\) \\([^ \n]+\\)\\( ([^)]*)\\)?[^\n]*" "*** def\\1\t\\2\\3") > ("^;;;" "***") > ("^;;" " ") > ("^ +$" "") > ("\n\n+" "\n\n"))) It doesn't really matter, admittedly, but it does seem like we could avoid the intermediate step of adding the semi-colons at the beginning of "all" the lines only to remove them right after. > (goto-char (point-min)) > (while (re-search-forward (car repl) nil t) > (replace-match (cadr repl)))) If we keep the two-step approach, then you'll probably want to define an auxiliary function which takes a list of regexp+replacement and does the searches+replacements. > ;; Create top heading > (goto-char (point-min)) > (delete-char 1) Which char do we intend to delete here and why? > ;; Create colophon heading > (forward-line 1) > (insert "** Colophon:\n") I see you don't use the top-level "*" headers at all. Any specific reason for that? > ;; Ta-da! > (goto-char (point-min)) > (org-mode) > (org-cycle) ; open up first-level headings I'd do the `goto-char` after setting up org-mode, just in case org-mode moves point: I know it arguably shouldn't/won't, but it doesn't cost anything to switch the two and it saves us from having to worry about it. Also, if `org-cycle` may open, but it may do other things as well (it's a DWIM command meant for interactive use), so in ELisp code we're better off using a lower-level function which only does "open up first-level headings", which should also save us from having to write a comment explaining what we're intending to do. > (when (re-search-forward "^\*\* Commentary:" nil t) These backslashes don't do any good here (as evidenced by the red warning faced applied to the by font-lock ;-). You probably intended for them to be doubled. > ;; open up content of anny commentary text ^^^^ short for anniversary?