From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Thien-Thi Nguyen Newsgroups: gmane.emacs.devel Subject: Re: [ELPA] New package: project-shells Date: Thu, 09 Mar 2017 09:09:56 +0100 Message-ID: <87shmm3m7f.fsf@zigzag.favinet> References: <87mvdbojds.fsf@163.com> <87varyd5eb.fsf@zigzag.favinet> <8737eu1qa7.fsf@163.com> Reply-To: emacs-devel@gnu.org NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha1; protocol="application/pgp-signature" X-Trace: blaine.gmane.org 1489046716 14490 195.159.176.226 (9 Mar 2017 08:05:16 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Thu, 9 Mar 2017 08:05:16 +0000 (UTC) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.0.50 (gnu/linux) To: emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Thu Mar 09 09:05:10 2017 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1clt4O-0002xS-10 for ged-emacs-devel@m.gmane.org; Thu, 09 Mar 2017 09:05:08 +0100 Original-Received: from localhost ([::1]:60899 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1clt4U-0001s3-0h for ged-emacs-devel@m.gmane.org; Thu, 09 Mar 2017 03:05:14 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:37713) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1clt4L-0001pX-2N for emacs-devel@gnu.org; Thu, 09 Mar 2017 03:05:06 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1clt4H-0007X3-T3 for emacs-devel@gnu.org; Thu, 09 Mar 2017 03:05:05 -0500 Original-Received: from mail.agora-net.com ([67.59.132.6]:59985) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1clt4H-0007Vw-PM for emacs-devel@gnu.org; Thu, 09 Mar 2017 03:05:01 -0500 Original-Received: from ttn by mail.agora-net.com with local (Exim 4.82) (envelope-from ) id 1clt4F-0002ju-M0 for emacs-devel@gnu.org; Thu, 09 Mar 2017 03:04:59 -0500 Original-Received: from ttn by zigzag.favinet with local (Exim 4.80) (envelope-from ) id 1clt9H-0001y6-PO for emacs-devel@gnu.org; Thu, 09 Mar 2017 09:10:11 +0100 Mail-Followup-To: emacs-devel@gnu.org In-Reply-To: <8737eu1qa7.fsf@163.com> (Ying Huang's message of "Fri, 03 Mar 2017 20:46:40 +0800") X-SA-Exim-Connect-IP: X-SA-Exim-Mail-From: ttn@gnuvola.org X-SA-Exim-Scanned: No (on mail.agora-net.com); SAEximRunCond expanded to false X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 67.59.132.6 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 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.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:212855 Archived-At: --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable () "Huang, Ying" () Fri, 03 Mar 2017 20:46:40 +0800 ;; Version: 20170222 ;; Package-Version: 20170222 Reminder: Don't forget to update these fields. "Manage shell buffers of each project" Needs period (?., U+2E) at end. Similarly for other instances. type ('shell or 'term) This is slightly unconventional. You explicitly name the type of the other alist components, why not this one as well? Maybe: TYPE (a symbol, either =E2=80=98shell=E2=80=99 or =E2=80=98term=E2=80=99) Another convention is to use all uppercase for metavariables. For example, see =E2=80=98auto-mode-alist=E2=80=99. (Thus, "TYPE" above.) One shell will be created for each key. Usually these key will be bound in a non-global keymap." I suggest naming the function that does this action. For example, "Normally, =E2=80=98project-shells-setup=E2=80=99 arranges for the= se keys to open a project-specific shell.". Naming the function (w/ proper quotes) creates a hyperlink in the *Help* buffer that helps the reader form a mental model of the package operation. The "Normally" serves two purposes: (a) It sidesteps the decision to choose between capitalizing "project-shells-setup", the first word in the sentence (correct from a grammar pov) and leaving it as-is (correct from an accuracy pov). (b) It invites follow-on documentation for any specific customization tips. I think (a) is a nice (technical writing) hack only, but (b) is crucial to the user experience (and maintenance burden) of this package. (defcustom project-shells-term-keys '("-" "=3D") "Keys used to create terminal buffers. By default shell mode will be used, but for keys in =E2=80=98project-shells-term-keys=E2=80=99, ansi terminal mode will be u= sed. This should be a subset of *poject-shells-keys*." Spelling error: "poject". Also, don't use asterisk (?*, U+2A) to quote API elements. Instead, use either backtick (?`, U+60) and apostrophe (?', U+27) or Unicode chars LEFT SINGLE QUOTATION MARK (?=E2=80=98, U+2018) and RIGHT SINGLE QUOTATION MARK (?=E2=80=99, U+20= 19). Lastly, design questions (rhetorical): What happens if "should be a subset" condition does not hold? Have you considered combining =E2=80=98project-shells-term-keys=E2=80=99 and =E2=80=98project-s= hell-keys=E2=80=99 somehow? What if my project supports live-hacking via REPL? (let ((saved-shell-buffer-list nil) (last-shell-name nil)) (cl-defun project-shells--buffer-list ...) (cl-defun project-shells--switch ...) (cl-defun project-shells-switch-to-last ...) (cl-defun project-shells--create ...)) Nice, much less scary for old eyes to read now. (I added another blank line prior to =E2=80=98project-shells--buffer-list=E2=80=99, = btw.) (cl-defun project-shells-send-shell-command (cmdline) "Send the command line to the current (shell) buffer. Can be used in shell initialized function." Move the "Can be..." sentence to the next line. More tips at: (info "(elisp) Documentation Tips") (let* ((key (replace-regexp-in-string "/" "slash" key)) ... (session-dir (expand-file-name (format "%s/%s" proj key) project-shells-session-root))) This is is why (b) above is important. This code special-cases slash (?/, U+2F), but will probably give surprising results for =E2=80=98\M-g=E2=80=99 or =E2=80=98\C-/=E2=80=99 and so on. You can either= add handling for those here, or document the range of "acceptable" =E2=80=98key=E2=80=99 val= ues. If you don't, you will receive complaints from users who try to use project-shells.el w/ strange (but valid) keys. I don't know about you, but i get enough complaints as it is... :-D =2D-=20 Thien-Thi Nguyen ----------------------------------------------- (defun responsep (query) (pcase (context query) (`(technical ,ml) (correctp ml)) ...)) 748E A0E8 1CB8 A748 9BFA =2D-------------------------------------- 6CE4 6703 2224 4C80 7502 --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) iEYEARECAAYFAljBDdkACgkQZwMiJEyAdQKLTgCfTmkqM99go7AZdfl7oAZdzxQl jSYAn1Nw/x1V0aJHMmmUTzXkAYIHbZJF =glDf -----END PGP SIGNATURE----- --=-=-=--