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: How to contribute new package to GNU ELPA? Date: Sat, 19 Dec 2020 10:35:37 -0500 Message-ID: References: <15c3cc00-f56e-6e52-2228-30817639315a@gmail.com> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="33003"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Cc: Emacs developers To: stardiviner Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Sat Dec 19 16:36:29 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 1kqeHV-0008TZ-Gf for ged-emacs-devel@m.gmane-mx.org; Sat, 19 Dec 2020 16:36:29 +0100 Original-Received: from localhost ([::1]:35270 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kqeHU-00052k-Ja for ged-emacs-devel@m.gmane-mx.org; Sat, 19 Dec 2020 10:36:28 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:33650) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kqeGs-0004cD-FV for emacs-devel@gnu.org; Sat, 19 Dec 2020 10:35:50 -0500 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:24699) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kqeGp-0007Fx-Md for emacs-devel@gnu.org; Sat, 19 Dec 2020 10:35:49 -0500 Original-Received: from pmg1.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id C823C100229; Sat, 19 Dec 2020 10:35:45 -0500 (EST) Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id E82C41000CF; Sat, 19 Dec 2020 10:35:43 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1608392143; bh=sTUfnVi8gwoKGrDtFfbclg4frZyYPEgIJkOgMZhaz2Y=; h=From:To:Cc:Subject:References:Date:In-Reply-To:From; b=HsQ1rko1FY+te42l/XQdHP/Jz2rhp2QbQicVNCoNESKHl73l4OPjuL7SxD6s8R1he DE35NbEZ3K/cIU4KdcYdUNsynNXdcvKnK/C3R1q0CzfCKdYOeS4j5afpq6Yf/JrPHa ziYuy0oUPg1knjlctPmk9IISmtEg6KG/wLIFULwtcgcJL+DBUvccaiWrg5bAp11F/K gb3zC1wHCBQCbWvD1EtKilIeiPsSfIgwMx9+j+sZREMFIChj7K8G8aMcjpF5uXz8RG dPUc/fHD+p5OdgG0eJkML0C8R//1xCE2fz8XIilIwJ21fHPxF69AFG7y7AueZ6GBEY E4X8x4aL1KYzA== Original-Received: from alfajor (69-165-136-52.dsl.teksavvy.com [69.165.136.52]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id B1C601204A2; Sat, 19 Dec 2020 10:35:43 -0500 (EST) In-Reply-To: <15c3cc00-f56e-6e52-2228-30817639315a@gmail.com> (stardiviner's message of "Sat, 19 Dec 2020 15:08:59 +0800") Received-SPF: pass client-ip=132.204.25.50; envelope-from=monnier@iro.umontreal.ca; helo=mailscanner.iro.umontreal.ca 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:261250 Archived-At: Hi >> -;; Package-Requires: ((emacs "24.4") (cl-lib "0.5") (request "0.3.0")) >> +;; Package-Requires: ((emacs "24.4") (request "0.3.0")) > > cl-lib should be required for `cl-function`. cl-lib comes bundles with Emacs>=24.3, so your Emacs-24.4 dependency already ensures cl-lib is available. >> ;; This file is part of GNU Emacs. >> @@ -28,13 +28,13 @@ >> ;;; Commentary: >> -;;; This currently only works for Linux, not tested for Mac OS >> X and Windows. >> +;; This currently only works for Linux, not tested for Mac OS X and Windows. >> -;;; Kiwix installation >> +;;;; Kiwix installation >> ;; >> ;; http://www.kiwix.org >> -;;; Config: >> +;;;; Config: >> ;; >> ;; (use-package kiwix >> ;; :ensure t >> @@ -45,7 +45,7 @@ >> ;; kiwix-server-port 8080 >> ;; kiwix-default-library "wikipedia_zh_all_2015-11.zim")) >> -;;; Usage: >> +;;;; Usage: > > Why use four `;` instead of three `;`? I search many packages, they all use > three `;`. I don't know which one is the standard. Because it's a sub-heading of "Commentary:" (e.g. we'd use five semi-colons for subsubheadings). >> +(if (featurep 'ivy) (require 'ivy)) ;FIXME: That's a no-op! > What does the "no-op" mean? Because some user might have not installed ivy, > so I added one condition to detect it here. If (featurep 'ivy) is non-nil, then (require 'ivy) won't do anything. I think your (featurep 'ivy) intended to test is Ivy is installed, whereas it tests if Ivy is already loaded. But in any case you don't need any of that: if Ivy is installed, then `ivy-read` will be auto-loaded, so the rest of your code will "just work" regardless if Ivy is already loaded or not. You might want to use something like: (defcustom kiwix-default-completing-read (cond ((fboundp 'ivy-read) 'ivy) ((fboundp 'helm) 'helm)) OTOH. > Why deleted all `:group 'kiwix-mode`? If think correct, the :group is used > by `customize-group`. So It should be necessary. These are redundant because by default vars are placed in the group that was last `defgroup`d. >> (defun kiwix-dir-detect () >> "Detect Kiwix profile directory exist." >> - (let ((kiwix-dir (concat (getenv "HOME") "/.www.kiwix.org/kiwix"))) >> - (if (file-accessible-directory-p kiwix-dir) >> + (let ((kiwix-dir "~/.www.kiwix.org/kiwix")) >> + (if (file-directory-p kiwix-dir) > Use `file-accessible-directory-p` because also can detect folder > permission. If user can't read folder, that's a problem too. Then use something better (e.g. (and (file-directory-p kiwix-dir) (file-readable-p kiwix-dir))), but (file-accessible-directory-p kiwix-dir) returns non-nil when the directory is absent, in which case the user can't read the directory either, so it's not testing what you want. > I prefer to use `$HOME` environment variable, it should be more > cross-platformed. Fine by me. I just thought it was an "obvious simplification". Actually, it's the other way around: HOME is just an env-var and its meaning is defined by the platform (it just happens to work fine with the currently supported platforms), whereas "~/" is defined by Emacs so we (Emacs maintainers) have to make sure it works on all platforms. ;-) >> :success (cl-function >> - (lambda (&key data &allow-other-keys) >> + (lambda (&key _data &allow-other-keys) ;FIXME: Why not `&rest _'? > For later success message output. But that's another function, so it's unrelated, AFAICT. So, I guess what you mean is that you put it for documentation purposes, to remind the reader what args are passed to the success function (and ignored in this particular success function). FWIW, I find this to be a perfectly good reason (even thought it costs a bit extra because the `cl-lib` will actually "work" to extract the `data` argument even though it's not used). >> -(defun kiwix-at-point (&optional interactively) >> +(defun kiwix-at-point () >> "Search for the symbol at point with `kiwix-query'. >> Or When prefix argument `INTERACTIVELY' specified, then prompt >> for query string and library interactively." >> - (interactive "P") >> + (interactive) > This is necessary for later command definition `kiwix-at-point-interactive`. No it's not. What this did is add an `interactively` argument and pass it the value of `current-prefix-arg`. Since you don't use the variable `interactively`, you don't need that: the variable `current-prefix-arg` is not affected by the above changes. This said, maybe I'm missing something: I don't see any place where you implement the "When prefix argument `INTERACTIVELY' specified" test, instead it seems your code just always "prompts for query string and library interactively". More importantly: what do you want to do about `request`? Stefan