Re: [Suggest] please include pyim-basedict ot nongnu elpa.

[Stefan Monnier <monnier@iro.umontreal.ca>]

Hi,

> pyim-basedict include importent pinyin dict for pyim.  it will significant enhance pyim.
> it is imported from libpinyin project, which is GPL3, but I can deal with GPL paper's problem.
> so I suggest put it to nongnu elpa.

I'm looking at it and have a few comments/questions:

- See patch below for some minor tweaks to the code

- When we include a file generated from elsewhere, I think it's very
  important that we make it clear how to re-generate it.  I see you
  include the code used to generate the file, but you don't say
  precisely where the source files can be found (you just state vaguely
  that they come from the libpinyin project).
  AFAICT, they come from
  http://downloads.sourceforge.net/libpinyin/models/model19.text.tar.gz, right?
  If so, that URL should appear in a comment somewhere (ideally within
  `pyim-basedict.pyim` which should also state the name of the function
  that created it, but maybe it's convenient to do that).

- While I see GPLv3 blurbs at various places in `libpinyin`, I don't see
  any license whatsoever in that `model19.text.tar.gz` file.
  So strictly speaking, I don't think we have any legal right to
  distribute that data.  I suspect that it is not the intention
  of those who maintain that model19 dataset, but we need to contact
  them so they clarify the license that applies to that dataset (they
  should include some kind of README file stating the license under
  which they make this data available).

- AFAICT you don't actually use any code from `libpinyin` right?
  All I see in `pyim-basedict` is code that you wrote plus the actual
  dictionary generated from the model19 data.
  If so, I think it could go into GNU ELPA once the licensing issue
  above is cleared (we don't need the copyright paperwork for the
  dataset itself).

- I see in the model19 an `interpolation2.text` file which your code
  doesn't seem to use.  I'm curious what it's about?


        Stefan




diff --git a/pyim-basedict.el b/pyim-basedict.el
index 42048a3eb..96c25f069 100644
--- a/pyim-basedict.el
+++ b/pyim-basedict.el
@@ -1,4 +1,4 @@
-;;; pyim-basedict.el --- The default pinyin dict of pyim
+;;; pyim-basedict.el --- The default pinyin dict of pyim  -*- lexical-binding: t; -*-
 
 ;; * Header
 ;; Copyright (C) 2015 Feng Shu <tumashu@163.com>
@@ -42,9 +42,8 @@
 ;; ** 安装和使用
 ;; 1. 配置melpa源，参考：http://melpa.org/#/getting-started
 ;; 2. M-x package-install RET pyim-basedict RET
-;; 3. 在emacs配置文件中（比如: ~/.emacs）添加如下代码：
+;; 3. 在Emacs配置文件中（比如: ~/.emacs）添加如下代码：
 ;;    #+BEGIN_EXAMPLE
-;;    (require 'pyim-basedict)
 ;;    (pyim-basedict-enable)
 ;;    #+END_EXAMPLE
 
@@ -73,17 +72,22 @@
 (defun pyim-basedict-enable ()
   "Add basedict to pyim."
   (interactive)
-  (let* ((file (concat (file-name-directory
-                        (locate-library "pyim-basedict.el"))
-                       "pyim-basedict.pyim")))
+  (let* ((file (expand-file-name "pyim-basedict.pyim"
+                                 (file-name-directory
+                                  (locate-library "pyim-basedict.el")))))
     (when (file-exists-p file)
+      ;; FIXME: If `pyim-basedict-enable' is called early enough, pyim
+      ;; won't be loaded yet and this (featurep 'pyim) will return nil.
+      ;; Maybe we should just (require 'pyim) and call
+      ;; `pyim-extra-dicts-add-dict' unconditionally, or maybe we should
+      ;; use `with-eval-after-load'.
       (if (featurep 'pyim)
           (pyim-extra-dicts-add-dict
            `(:name "Basedict-elpa"
-                   :file ,file
-                   :coding utf-8-unix
-                   :dict-type pinyin-dict
-                   :elpa t))
+             :file ,file
+             :coding utf-8-unix
+             :dict-type pinyin-dict
+             :elpa t))
         (message "pyim 没有安装，pyim-basedict 启用失败。")))))