DocViewModeにpatchを送りEmacs contributorになるまで

前書き

やっほ! [http://twitter.com/laysakura[@laysakura]] です!
http://d.hatena.ne.jp/laysakura/20110326/1301168759 の記事にも書いたのですが，最近は Emacs の DocViewMode の patch 書きに腐心していました．
それで色々ありまして，Emacs の contributor に名を連ねることが出来そうなので，それを記事にしました．

自分自身，オープンソースな世界に初めて足を踏み入れて戸惑った&勘違いしたことが多々ありますので，僕の記事を読んで少しでもオープンソースに関わる方が増えれば幸いです．

※文中にソースコードが出てきますが，日本語だけ読んでも大丈夫です．

DocViewMode を少しご紹介

DocViewMode は，Emacs23 以降に標準で入っている機能です．
C-x C-f で PDF や PS ファイルを開くと，バックグラウンドで PNG に変換されてそれが Emacs のバッファに表示されるという素敵なやつです．

事の始まり

WYSIWYG-TeX.el (<-オススメ) を作っている際に，DocViewMode に追加機能が欲しくなりました．
表示している画像を，ウィンドウサイズに合わせて拡大・縮小できるようにしたかったのです．
それを実現するべく，とりあえず自分の環境で動くコードを書きました．

こんな感じです(読まなくてもおkです)

;;; Fit-to-page extension for DocViewMode

(require 'doc-view)

(defconst doc-view-tmp-png-path "/tmp/doc-view-tmp.png")
(defconst doc-view-tmp-pdf/ps-path "/tmp/doc-view-tmp-img")

(defun doc-view-get-dpi ()
  "Returns resolution of your display in Dpi.
If you don't use X Window System, this function asks you Dpi."
  (let ((resolution-info (shell-command-to-string
                          "xdpyinfo |grep 'resolution'")))
    (if (string-match "\\([0-9]+\\)x" resolution-info)
        (string-to-number (match-string 1 resolution-info))
      (read-number "Input your display resolution (Dpi): "))))
;; (doc-view-get-dpi)

(defun doc-view-pdf/ps-size-in-px (pdf/ps-path)
  "Returns (width . height) of PDF/PS file with 'pdf/ps-path`"
  (call-process "gs" nil nil t
                "-dSAFER" "-dNOPAUSE" "-sDEVICE=png16m" "-dTextAlphaBits=4"
                "-dBATCH" "-dGraphicsAlphaBits=4" "-dQUIET"
                "-dFirstPage=1" "-dLastPage=1" (concat "-sOutputFile=" doc-view-tmp-png-path)
                pdf/ps-path)
  (cons (string-to-number
         (shell-command-to-string
          (concat "echo -n `identify -format \"%w\" " doc-view-tmp-png-path "`")))
        (string-to-number
         (shell-command-to-string
          (concat "echo -n `identify -format \"%h\" " doc-view-tmp-png-path "`")))))
;; (doc-view-pdf/ps-size-in-px "sample.ps")
;; (doc-view-pdf/ps-size-in-px "sample.pdf")

(defun doc-view-window-size-in-px ()
  "Returns current window size in pixel like (width . height) ."
  (cons (* (window-width) (frame-char-width))
        (* (window-height) (frame-char-height))))
;; (doc-view-window-size-in-px)

(defun doc-view-px-to-natural (px dpi)
  "Convert pixel to \"Natural size\", which is about the same size with one printed on paper.
Emacs displays images with \"Natural size\", so this conversion is necessary."
  (* px 1.3871979865e-2 dpi))
;; (doc-view-px-to-natural 596 96)

(defun doc-view-percentage-to-fit (pdf/ps-path dpi)
  "Returns percentage you can set to fit image onto a window.
For example, (88 . 74) means if you set 88 to 'doc-view-resolution',
the width of the image fits the window."
  (let* ((pdf/ps-size (doc-view-pdf/ps-size-in-px pdf/ps-path))
         (pdf/ps-width (car pdf/ps-size))
         (pdf/ps-height (cdr pdf/ps-size))
         (window-size (doc-view-window-size-in-px))
         (window-width (car window-size))
         (window-height (cdr window-size)))
    (cons (* 100.0 (/ window-width (doc-view-px-to-natural (float pdf/ps-width) dpi)))
          (* 100.0 (/ window-height (doc-view-px-to-natural (float pdf/ps-height) dpi))))))
;; (doc-view-percentage-to-fit "sample.ps" (doc-view-get-dpi))
;; (doc-view-percentage-to-fit "sample.pdf" (doc-view-get-dpi))

(defun doc-view-change-size (size)
  "Changes the size of the image viewed in DocViewMode into 'size'."
  (set (make-local-variable 'doc-view-resolution) size)
  (doc-view-reconvert-doc))

(defun doc-view-fit-height ()
  "Makes the height of image fit with window."
  (interactive)
  (doc-view-change-size
   (round (cdr (doc-view-percentage-to-fit (buffer-file-name) (doc-view-get-dpi))))))
(defun doc-view-fit-width ()
  "Makes the height of image fit with window."
  (interactive)
  (doc-view-change-size
   (round (car (doc-view-percentage-to-fit (buffer-file-name) (doc-view-get-dpi))))))
(defun doc-view-fit-page ()
  "Makes image fit with window."
  (interactive)
  (let ((percentage (doc-view-percentage-to-fit (buffer-file-name) (doc-view-get-dpi))))
    (doc-view-change-size (round (min (car percentage)
                             (cdr percentage))))))

(provide 'doc-view-fit-page)

これを提案内容の説明と共に，作者の方にメールで送りました．
もちろん，文面に失礼がないように・しっかりと伝えたいことが伝わるように，何回か見直しました .

作者ツン期

オープンソースでコードを書くときに心掛けること

実は送った段階では華麗にスルーされるだろうなと8割方思っていたのですが，24時間以内にお返事が返ってきました．
お返事の内容の技術的な部分を要約しますと，

アイディアはいいね．
でもこのコードは不完全だ．採用できないよ．
第一に，環境に依存するコードを書きすぎている．
第二に，折角こちらが用意した便利な再描画用の関数を使用していない．

ということでした．

環境に依存するコードを書きすぎているの方は完全に盲点でした．
普段は自分の Linux 上で予定通り動いたらOKとしていましたが，知らず知らずのうちに他の環境に対する配慮を欠いていた訳です．
裏でシェルを走らせるような処理を直接書くのは，本当にそれでポータビリティが失われていないかを重々検討してからじゃないといけないと認識を改めました．

既にあるコードを有効利用できていないという指摘も非常に的を射ていました．
「こうすればできるだろう」という処理が頭に浮かんでからはそれをコードに落とし込むことばかりを考え，既にあるコードでも同じ事をしていないかを確認せず，同じコードを2度書くなという大原則を破ってしまっていました．
もちろん自分の書いた範囲では無駄なコードは書いていないはずなのですが，後にソースを読んだところ，やはり作者のコードには自分の書いたことが，よりベターに書かれていました．

オープンソースプロジェクトに patch を送る手順

また，作者にはプロジェクトへ patch を送る方法を丁寧に教えていただきました．
今回は GNU Emacs の部分プロジェクトですので，それに固有な話も多少はあると思いますが，根本的にやることはどのプロジェクトでも同じはずですので参考にしてください．

バージョン管理システムを用いて，最新の開発版のソースコードを入手
- http://www.m17n.org/mlarchive/mule-ja/201001/msg00000.html を参考にし，以下の手順で GNU Emacs の最新版のソースを入手しました．

sudo apt-get install bzr
bzr whoami "Sho Nakatani <xxx@yyy.com>"
cd ~/src
bzr init-repo emacs/
cd emacs
bzr branch http://bzr.savannah.gnu.org/r/emacs/trunk trunk
cd trunk
echo "public_branch = http://bzr.savannah.gnu.org/r/emacs/trunk"; >> .bzr/branch/branch.conf
bzr bind http://bzr.savannah.gnu.org/r/emacs/trunk

最新のテスト環境を得るために，ソースをビルドする
- 初めは今まで通り Emacs23 を使ってコードを書いていたのですが，意味不明なバグにやられました．原因は， doc-view.el が使用しているある関数が最新の Emacs にしか入っていないことでした．このように時間を浪費しないためにも，ちゃんと最新のソースをビルドして，その環境でテストをしましょう．

./autogen.sh
./configure  (失敗)
sudo apt-get isntall libgif-dev
./configure
bzr pull
make bootstrap

自分のやりたいことに関わるコードはしっかり読む
- 重要です．これをしなければ，別の人が書いた処理を再び別の場所に書く事になりかねません．そればかりか，自分が思いつきで書く処理よりも上手な処理がもう書かれているということが十分あり得ます．作者の方が時間を掛けているでしょうから当然ですね．

オリジナルを残しつつ，お試し用のファイルでソースを書き換えていく
- patch を送るとは，要するにオリジナルと改変後の diff 結果を送るということです．オリジナルをそのまま残し，お試し用のコピーファイルでガシガシ変更を加えていき，お試し用のが完成したら追加・修正した部分を注意深くオリジナルに反映させていくという手順を踏むのが良いと思われます．もちろんこの手順はバージョン管理システムを駆使して行うこともできます．

オリジナルに修正を反映させる
- この際に，以下の点に最低限注意します．
  - diff 結果に余計なものが表示されない様にする．改行・スペース・タブなども勝手に追加・削除してはいけません．
  - コード全体が取っているコーディング規約を感じ取り，遵守するように努める．

patch の作成
- patch は要するに diff 結果だと述べましたが，それだけでなく，

patch -p1 [オリジナルファイル] < [パッチファイル]

の1コマンドによって [オリジナルファイル] が自分の書き換えたとおりに再現できます．
bzr の場合は，以下のコマンドで patch を作成します．

bzr diff -p1 > fit-to-window.patch

もちろん確認も忘れずに!

patch -p1 lisp/doc-view.el < fit-to-window.path

再実装

作者のメールから得た反省点を活かすように意識しつつコードを書き直し，完成したパッチを送りました．
これがパッチの内容です．

=== modified file 'lisp/doc-view.el'
--- old/lisp/doc-view.el	2011-01-25 04:08:28 +0000
+++ new/lisp/doc-view.el	2011-03-28 08:59:52 +0000
@@ -327,6 +327,10 @@
     ;; Zoom in/out.
     (define-key map "+"               'doc-view-enlarge)
     (define-key map "-"               'doc-view-shrink)
+    ;; Fit the image to the window
+    (define-key map "W"               'doc-view-fit-width-to-window)
+    (define-key map "H"               'doc-view-fit-height-to-window)
+    (define-key map "P"               'doc-view-fit-page-to-window)
     ;; Killing the buffer (and the process)
     (define-key map (kbd "k")         'doc-view-kill-proc-and-buffer)
     (define-key map (kbd "K")         'doc-view-kill-proc)
@@ -665,6 +669,45 @@
   (interactive (list doc-view-shrink-factor))
   (doc-view-enlarge (/ 1.0 factor)))
 
+(defun doc-view-fit-width-to-window ()
+  "Fit the image width to the window width."
+  (interactive)
+  (let ((img-width (car (image-display-size
+                         (image-get-display-property))))
+        (win-width (- (nth 2 (window-inside-edges))
+                      (nth 0 (window-inside-edges)))))
+    (doc-view-enlarge (/ win-width img-width))))
+
+(defun doc-view-fit-height-to-window ()
+  "Fit the image height to the window width."
+  (interactive)
+  (let ((img-height (cdr (image-display-size
+                          (image-get-display-property))))
+        (win-height (- (nth 3 (window-inside-edges))
+                       (nth 1 (window-inside-edges)))))
+    ;; When users call 'doc-view-fit-height-to-window',
+    ;; they might want to go to next page by typing SPC
+    ;; ONLY once. So I used '(- win-height 1)' instead of
+    ;; 'win-height'
+    (doc-view-enlarge (/ (- win-height 1) img-height))))
+
+(defun doc-view-fit-page-to-window ()
+  "Fit the image to the window.
+More specifically, this function enlarges image by:
+
+min {(window-width / image-width), (window-height / image-height)} times."
+  (interactive)
+  (let ((img-width (car (image-display-size
+                         (image-get-display-property))))
+        (win-width (- (nth 2 (window-inside-edges))
+                      (nth 0 (window-inside-edges))))
+        (img-height (cdr (image-display-size
+                          (image-get-display-property))))
+        (win-height (- (nth 3 (window-inside-edges))
+                       (nth 1 (window-inside-edges)))))
+    (doc-view-enlarge (min (/ win-width img-width)
+                           (/ (- win-height 1) img-height)))))
+
 (defun doc-view-reconvert-doc ()
   "Reconvert the current document.
 Should be invoked when the cached images aren't up-to-date."

一番最初に作者に送った提案に比べて，変更点が非常に短くなっていることが分かるかと思います．
しかもそのくせ，元の提案版よりも余程良い動作をするようになりました．
ソースをしっかり読めば読むほど，変更箇所は少なくできると感じました．

作者デレ期

上記パッチを作者にお送りしたところ，一転してデレてきました．

素晴らしいパッチだ!
テストもちゃんと通ったよ． emacs-dev のメーリングリストに君のパッチを報告しておくよ!
それじゃあ，まず http://www.gnu.org/licenses/why-assign.html を読んで， FSFに著作権を委譲することに同意してね．
同意できたら， http://git.savannah.gnu.org/cgit/gnulib.git/tree/doc/Copyright/request-assign.future のフォームを埋めて， assign@gnu.org に送ってね．
それで FSF から君に確認が来たら，正式に patch を反映したソースを push するよ．
そしたら君も晴れて Emacs contributor だ!