読者です 読者をやめる 読者になる 読者になる

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 の部分プロジェクトですので,それに固有な話も多少はあると思いますが,根本的にやることはどのプロジェクトでも同じはずですので参考にしてください.

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 だ!

嬉しす

フォームの埋め方が非常に不安だったので, 作者さんに添削をお願いしました . 案の定, 数カ所問題点を見つけていただきました

今はフォームを送り,FSF からの連絡待ちです.

後書き

大体こんな感じでこの2,3日は過ぎていきました.
何しろ初めてのことばかりで,神経を使う場面もありましたが,非常に得難い経験ができたと思います.

何より, コードを書くという作業が,ソーシャライズされることで何倍も刺激的になる ということを肌で感じられたのが一番の収穫です.

今は emacs-dev ML に作者が patch を投稿してくれた結果,新たに出てきた要望を実装しようとしているところですが,これも楽しみながらやれそうです.

長くなりましたが,この辺でこの記事を終えたいと思います.
Be Social !!