From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: stardiviner Newsgroups: gmane.emacs.devel Subject: Re: How to contribute new package to GNU ELPA? Date: Sat, 19 Dec 2020 15:08:59 +0800 Message-ID: <15c3cc00-f56e-6e52-2228-30817639315a@gmail.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="5559"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 Cc: Emacs developers To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Sat Dec 19 08:10:11 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 1kqWNW-0001Lj-M3 for ged-emacs-devel@m.gmane-mx.org; Sat, 19 Dec 2020 08:10:10 +0100 Original-Received: from localhost ([::1]:38264 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kqWNV-0004io-Jb for ged-emacs-devel@m.gmane-mx.org; Sat, 19 Dec 2020 02:10:09 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:51830) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kqWMX-0004Hi-9d for emacs-devel@gnu.org; Sat, 19 Dec 2020 02:09:09 -0500 Original-Received: from mail-pl1-x62c.google.com ([2607:f8b0:4864:20::62c]:34796) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kqWMU-0001xi-W6 for emacs-devel@gnu.org; Sat, 19 Dec 2020 02:09:08 -0500 Original-Received: by mail-pl1-x62c.google.com with SMTP id t6so2634683plq.1 for ; Fri, 18 Dec 2020 23:09:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding:content-language; bh=CxNw3I0Mi0Bdy44W+3Iz75x8GJ4KpygkS4NHr/duoGA=; b=P8yeyhakfrlq4cqnIvsbV3VfzZ6H66hvNXdrZNZ7YG2jHTfggVQuYA2G8jDv/kbunC 469OHDGw5nOFCTKtnPXylSToqfaJgaw9bdu7zlbN4T+U2rzIu7vkSiO7cFDSk1I1KNUz Zm76cR5sh9efaxJX/+bvp1lfuuM+jOL1kVI+jNeZCbKqpxZSjxtLZMG1lV+N+1G5MA8f UKdrULqkYHN+0I8qpEfb8LY4aYgc4s44rkVF1H8zLFbylQvggnQrMIcLsWahgbQVZCBv qZdcWkleDIuTla+RzesWR6ixGbLIRaRxJMd4l5TBZAKooU1N92uwJzZYNZHLRdDT74rf YCww== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding :content-language; bh=CxNw3I0Mi0Bdy44W+3Iz75x8GJ4KpygkS4NHr/duoGA=; b=ru3h6v0CTaT4VEqdWxNTrNmEiOVjjtN6JyL4fyH2IQBNeKmbWPv5FF9iPxIaGqrgZE XCkFNBze59EOTsRSwtsBmF8WowpPWNQX9up8oOWUwdLUpPfx28TexIsIhaGRoBGMTV66 TBrQuEJhXP7V4ojZoH8qcqQTZsAm+x1ekJPZ7bosBZQ45bfojfTbGqKV9ULemdM8EWjl 2TrckFgmNia8f1NCtay2lJmKb15lRf3zPLTEOhIc2e5QVm8xltnU50g4L/dF5NBbjUVQ GkOTjBNvgcfpWY8tuIB3GpMVDOgtNIl4/1mvoL/LJsbz8P95AUamvYMeMlQvteoBiskm JwZg== X-Gm-Message-State: AOAM5303lRlZwoUxIoyWYMU+33spf2WsMXTu2goSOgxfZ8vu5gLXPcou +EHIceS7WpZ9keYHXtEXN+jRErB82ben X-Google-Smtp-Source: ABdhPJyXE+HqVnMAJ5eqjfJ0KCKK6bPLwGqQ3HvMTERZX3GnzXlkBlGmqi5vRxa7IHoHgoPfq8n6KA== X-Received: by 2002:a17:90a:ad7:: with SMTP id r23mr7626800pje.149.1608361744612; Fri, 18 Dec 2020 23:09:04 -0800 (PST) Original-Received: from [0.0.0.0] ([150.109.103.155]) by smtp.gmail.com with ESMTPSA id a11sm11118661pfr.198.2020.12.18.23.09.02 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 18 Dec 2020 23:09:03 -0800 (PST) In-Reply-To: Content-Language: en-US Received-SPF: pass client-ip=2607:f8b0:4864:20::62c; envelope-from=numbchild@gmail.com; helo=mail-pl1-x62c.google.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 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, FREEMAIL_FROM=0.001, NICE_REPLY_A=-0.001, RCVD_IN_DNSWL_NONE=-0.0001, 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:261234 Archived-At: On 2020/12/19 下午2:22, Stefan Monnier wrote: > stardiviner [2020-12-17 20:11:39] wrote: >> As this issue https://github.com/stardiviner/kiwix.el/issues/3 suggested. >> I would like to contribute my package kiwix.el to GNU ELPA. > I've just started to look at it. I have done some of the preliminary > work, but there are some problems to solve: > > 1- The package uses `request` which is neither in Emacs nor in GNU ELPA. > 2- The PNG images which are useful for the Homepage make the package > much larger for no good reason, so I think we should add > a `.elpaignore` file so as not to include those PNG imagines in the > package's tarball. > > 3- Even with `request`, compiling the package fails because > `org-kiwix.el` requirex `kiwix` and loading `kiwix` signal an error: > > (file-missing "Opening directory" "Aucun fichier ou dossier de ce type" "/.www.kiwix.org/kiwix") > > So, I think `file-accessible-directory-p` is not the function you > want to use. > > The patch below fixes some of those problems, but I still get warnings > about a missing .www.kiwix... directory when I load the file, which can > happen even if the user never intended to actually use any of that > package's functionality: those warnings should be delayed to when we > actually call some of the package's functions. > > The main problem, tho, is the `request` dependency. There are two > possible solutions to that: either we add `request` to GNU ELPA, or we > rewrite the code so as not to use `request` (IIUC `request` is > a reasonably thing wrapper above the `url` package, so it might not be > too bad to do). The best course is probably to add `request` to > GNU ELPA. Could you look into that, see how much work it would take to > include `request` in GNU ELPA? > > > Stefan > > > diff --git a/kiwix.el b/kiwix.el > index 5b765b5d3c..377772ee2e 100644 > --- a/kiwix.el > +++ b/kiwix.el > @@ -1,5 +1,5 @@ > -;;; kiwix.el --- Searching offline Wikipedia through Kiwix. > -;;; -*- coding: utf-8 -*- > +;;; kiwix.el --- Searching offline Wikipedia through Kiwix. -*- lexical-binding: t; -*- > +;; -*- coding: utf-8 -*- > > ;; Copyright (C) 2019-2020 Free Software Foundation, Inc. lexical-binding added. > > @@ -9,7 +9,7 @@ > ;; URL: https://github.com/stardiviner/kiwix.el > ;; Created: 23th July 2016 > ;; Version: 1.0.0 > -;; 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`. > > ;; 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. > ;; > ;; 1. [M-x kiwix-launch-server] to launch Kiwix server. > ;; 2. [M-x kiwix-at-point] to search the word under point or the region selected string. > @@ -58,7 +58,7 @@ > (require 'subr-x) > (require 'thingatpt) > (require 'json) > -(if (featurep 'ivy) (require 'ivy)) > +(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. > > (defgroup kiwix-mode nil > "Kiwix customization options." > @@ -67,19 +67,16 @@ > (defcustom kiwix-server-use-docker nil > "Using Docker container for kiwix-serve or not?" > :type 'boolean > - :safe #'booleanp > - :group 'kiwix-mode) > + :safe #'booleanp) > > (defcustom kiwix-server-port 8000 > "Specify default kiwix-serve server port." > :type 'number > - :safe #'numberp > - :group 'kiwix-mode) > + :safe #'numberp) > > (defcustom kiwix-server-url (format "http://127.0.0.1:%s" kiwix-server-port) > "Specify Kiwix server URL." > - :type 'string > - :group 'kiwix-mode) > + :type 'string) > > (defcustom kiwix-server-command > (cond > @@ -90,44 +87,40 @@ > ((string-equal system-type "windows-nt") > (warn "You need to specify Windows Kiwix path. And send a PR to my repo."))) > "Specify kiwix server command." > - :type 'string > - :group 'kiwix-mode) > + :type 'string) > Why deleted all `:group 'kiwix-mode`? If think correct, the :group is used by `customize-group`. So It should be necessary. > (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. I prefer to use `$HOME` environment variable, it should be more cross-platformed. > kiwix-dir > - (warn "ERROR: Kiwix profile directory \".www.kiwix.org/kiwix\" is not accessible.")))) > + (warn "ERROR: Kiwix profile directory \"~/.www.kiwix.org/kiwix\" is not accessible.") > + nil))) Applied > > (defcustom kiwix-default-data-profile-name > (when (kiwix-dir-detect) > - (car (directory-files > - (concat (getenv "HOME") "/.www.kiwix.org/kiwix") nil ".*\\.default"))) > + (car (directory-files "~/.www.kiwix.org/kiwix" nil ".*\\.default"))) Same as upper. > "Specify the default Kiwix data profile path." > - :type 'string > - :group 'kiwix-mode) > + :type 'string) > > (defcustom kiwix-default-data-path > (when (kiwix-dir-detect) > - (concat (getenv "HOME") "/.www.kiwix.org/kiwix/" kiwix-default-data-profile-name)) > + (expand-file-name kiwix-default-data-profile-name > + "~/.www.kiwix.org/kiwix/")) > "Specify the default Kiwix data path." > :type 'string > - :safe #'stringp > - :group 'kiwix-mode) > + :safe #'stringp) > Same as upper. > (defcustom kiwix-default-library-path (file-name-directory > (concat kiwix-default-data-path "/data/library/library.xml")) > "Kiwix libraries path." > :type 'string > - :safe #'stringp > - :group 'kiwix-mode) > + :safe #'stringp) > > (defcustom kiwix-default-completing-read 'ivy > "Kiwix default completion frontend. Currently Ivy ('ivy) and Helm ('helm) both supported." > :type 'symbol > - :safe #'symbolp > - :group 'kiwix-mode) > + :safe #'symbolp) > > (defcustom kiwix-default-browser-function browse-url-browser-function > "Set default browser for open kiwix query result URL." > @@ -139,8 +132,7 @@ > (const :tag "Google Chrome web browser" browse-url-chrome) > (const :tag "Conkeror web browser" browse-url-conkeror) > (const :tag "xwidget browser" xwidget-webkit-browse-url)) > - :safe #'symbolp > - :group 'kiwix-mode) > + :safe #'symbolp) > > ;;;###autoload > (defun kiwix--get-library-name (file) > @@ -179,19 +171,16 @@ Like in function `kiwix-ajax-search-hints'.") > (defcustom kiwix-default-library "wikipedia_en_all.zim" > "The default kiwix library when library fragment in link not specified." > :type 'string > - :safe #'stringp > - :group 'kiwix-mode) > + :safe #'stringp) > > (defcustom kiwix-search-interactively t > "`kiwix-at-point' search interactively." > :type 'boolean > - :safe #'booleanp > - :group 'kiwix-mode) > + :safe #'booleanp) > > (defcustom kiwix-mode-prefix nil > "Specify kiwix-mode keybinding prefix before loading." > - :type 'kbd > - :group 'kiwix-mode) > + :type 'kbd) > Same as upper. > ;; update kiwix server url and port > (defun kiwix-server-url-update () > @@ -258,7 +247,7 @@ Like in function `kiwix-ajax-search-hints'.") > (when (string-equal (cdr error-thrown) "exited abnormally with code 7\n") > (warn "kiwix.el failed to connect to host. exited abnormally with status code: 7.")))) > :success (cl-function > - (lambda (&key data &allow-other-keys) > + (lambda (&key _data &allow-other-keys) ;FIXME: Why not `&rest _'? For later success message output. > (setq kiwix-server-available? t))) > :status-code '((404 . (lambda (&rest _) (message (format "Endpoint %s does not exist." url)))) > (500 . (lambda (&rest _) (message (format "Error from %s." url)))))))) > @@ -290,12 +279,12 @@ list and return a list result." > (mapcar 'cdar data))))) > > ;;;###autoload > -(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`. > (unless (kiwix-ping-server) > (kiwix-launch-server)) > (if kiwix-server-available? > @@ -368,7 +357,6 @@ for query string and library interactively." > :require 'kiwix-mode > :init-value nil > :lighter " Kiwix" > - :group 'kiwix-mode > :keymap kiwix-mode-map > (if kiwix-mode (kiwix-mode-enable) (kiwix-mode-disable))) > > Same as upper.