背景
2023/11にEmacsのorg-modeにコントリビュートを行った。 OSSに対してGitHubのPull Requestでコントリビュートしたことは何度かあるが、Emacs / org-modeに対するコントリビュートは初めてで、さらにいうとメーリングリストでパッチを送るということ自体が初めてだった。
パッチの概要、時系列のタイムライン、この経験での学びを記載する。
パッチの概要
org-special-ctrl-a/e が tになっているという前提。
C-u 2 C-a のように前置引数が正の整数で org-beginning-of-line を実行すると、本当は見出し記号の直後にカーソルが移動することを期待するが、実際には行頭に移動してしまう。なお、C-u 0 C-aのようなファイル先頭方向に移動する際には期待通りの挙動を示す。
この挙動はmarkdown-modeに対して org-beginning-of-line と同等の機能を実装していくなかでの自動テストで発見した。
前置引数ありで org-beginning-of-line を呼び出し、それが運よく（運悪く？）見出しやリスト行に当たるということは少ないだろうから、markdown-mode向けの再実装をしなければ見つけられなかったと思う。
実際のパッチ
From 739c6636cd9e015ed214a6ccaed20cf75301a8d5 Mon Sep 17 00:00:00 2001 From: Tomohisa Kuranari <tomohisa.kuranari@gmail.com> Date: Fri, 22 Sep 2023 22:38:26 +0900 Subject: [PATCH] org-beginning/end-of-line: Fix when moving to different line * lisp/org.el (org-beginning-of-line, org-end-of-line): Fix issue with `org-special-ctrl-a/e' not working correctly when moving to different line * testing/lisp/test-org.el (test-org/beginning-of-line, test-org/end-of-line): Add new tests. --- lisp/org.el | 6 ++-- testing/lisp/test-org.el | 62 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/lisp/org.el b/lisp/org.el index d0b2355ea..7cd313c30 100644 --- a/lisp/org.el +++ b/lisp/org.el @@ -20331,7 +20331,7 @@ With argument N not nil or 1, move forward N - 1 lines first." (if (eq special 'reversed) (when (and (= origin bol) (eq last-command this-command)) (goto-char refpos)) - (when (or (> origin refpos) (= origin bol)) + (when (or (> origin refpos) (<= origin bol)) (goto-char refpos))))) ((and (looking-at org-list-full-item-re) (org-element-type-p @@ -20347,7 +20347,7 @@ With argument N not nil or 1, move forward N - 1 lines first." (if (eq special 'reversed) (when (and (= (point) origin) (eq last-command this-command)) (goto-char after-bullet)) - (when (or (> origin after-bullet) (= (point) origin)) + (when (or (> origin after-bullet) (>= (point) origin)) (goto-char after-bullet))))) ;; No special context. Point is already at beginning of line. (t nil)))) @@ -20402,7 +20402,7 @@ With argument N not nil or 1, move forward N - 1 lines first." (goto-char tags) (end-of-line))) (t - (if (or (< origin tags) (= origin (line-end-position))) + (if (or (< origin tags) (>= origin (line-end-position))) (goto-char tags) (end-of-line)))))) ((bound-and-true-p visual-line-mode) diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el index 8355e2d77..63fd16b40 100644 --- a/testing/lisp/test-org.el +++ b/testing/lisp/test-org.el @@ -4460,6 +4460,16 @@ asd (let ((org-special-ctrl-a/e '(nil . nil))) (org-beginning-of-line) (looking-at "Headline")))) + (should + (org-test-with-temp-text "* TODO [#A] Headline\n<point>" + (let ((org-special-ctrl-a/e t)) + (org-beginning-of-line 0) + (looking-at-p "Headline")))) + (should + (org-test-with-temp-text "<point>\n* TODO [#A] Headline" + (let ((org-special-ctrl-a/e t)) + (org-beginning-of-line 2) + (looking-at-p "Headline")))) ;; At an headline with reversed movement, first move to beginning of ;; line, then to the beginning of title. (should @@ -4480,6 +4490,18 @@ asd (this-command last-command)) (and (progn (org-beginning-of-line) (bolp)) (progn (org-beginning-of-line) (looking-at-p "Headline")))))) + (should + (org-test-with-temp-text "* TODO Headline\n<point>" + (let ((org-special-ctrl-a/e 'reversed) + (this-command last-command)) + (and (progn (org-beginning-of-line 0) (bolp)) + (progn (org-beginning-of-line) (looking-at-p "Headline")))))) + (should + (org-test-with-temp-text "<point>\n* TODO Headline" + (let ((org-special-ctrl-a/e 'reversed) + (this-command last-command)) + (and (progn (org-beginning-of-line 2) (bolp)) + (progn (org-beginning-of-line) (looking-at-p "Headline")))))) ;; At an item with special movement, first move after to beginning ;; of title, then to the beginning of line, rinse, repeat. (should @@ -4488,6 +4510,14 @@ asd (and (progn (org-beginning-of-line) (looking-at-p "Item")) (progn (org-beginning-of-line) (bolp)) (progn (org-beginning-of-line) (looking-at-p "Item")))))) + (should + (org-test-with-temp-text "- [ ] Item\n<point>" + (let ((org-special-ctrl-a/e t)) + (org-beginning-of-line 0) (looking-at-p "Item")))) + (should + (org-test-with-temp-text "<point>\n- [ ] Item" + (let ((org-special-ctrl-a/e t)) + (org-beginning-of-line 2) (looking-at-p "Item")))) ;; At an item with reversed movement, first move to beginning of ;; line, then to the beginning of title. (should @@ -4496,6 +4526,18 @@ asd (this-command last-command)) (and (progn (org-beginning-of-line) (bolp)) (progn (org-beginning-of-line) (looking-at-p "Item")))))) + (should + (org-test-with-temp-text "- [X] Item\n<point>" + (let ((org-special-ctrl-a/e 'reversed) + (this-command last-command)) + (and (progn (org-beginning-of-line 0) (bolp)) + (progn (org-beginning-of-line) (looking-at-p "Item")))))) + (should + (org-test-with-temp-text "<point>\n- [X] Item" + (let ((org-special-ctrl-a/e 'reversed) + (this-command last-command)) + (and (progn (org-beginning-of-line 2) (bolp)) + (progn (org-beginning-of-line) (looking-at-p "Item")))))) ;; Leave point before invisible characters at column 0. (should (org-test-with-temp-text "[[https://orgmode.org]]<point>" @@ -4598,6 +4640,14 @@ asd (and (progn (org-end-of-line) (looking-at-p " :tag:")) (progn (org-end-of-line) (eolp)) (progn (org-end-of-line) (looking-at-p " :tag:")))))) + (should + (org-test-with-temp-text "* Headline1 :tag:\n<point>" + (let ((org-special-ctrl-a/e t)) + (org-end-of-line 0) (looking-at-p " :tag:")))) + (should + (org-test-with-temp-text "<point>\n* Headline1 :tag:\n" + (let ((org-special-ctrl-a/e t)) + (org-end-of-line 2) (looking-at-p " :tag:")))) (should (org-test-with-temp-text "* Headline2a :tag:\n** Sub" (org-overview) @@ -4625,6 +4675,18 @@ asd (this-command last-command)) (and (progn (org-end-of-line) (eolp)) (progn (org-end-of-line) (looking-at-p " :tag:")))))) + (should + (org-test-with-temp-text "* Headline3 :tag:\n<point>" + (let ((org-special-ctrl-a/e 'reversed) + (this-command last-command)) + (and (progn (org-end-of-line 0) (eolp)) + (progn (org-end-of-line) (looking-at-p " :tag:")))))) + (should + (org-test-with-temp-text "<point>\n* Headline3 :tag:\n" + (let ((org-special-ctrl-a/e 'reversed) + (this-command last-command)) + (and (progn (org-end-of-line 2) (eolp)) + (progn (org-end-of-line) (looking-at-p " :tag:")))))) (should (org-test-with-temp-text "* Headline2a :tag:\n** Sub" (org-overview) -- 2.42.0
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=93ebd64de
タイムライン
- 2023/08/05
- Copyright Assignmentのリクエスト
- 2023/08/15
- AssignmentのPDFを受信(from kuranari)
- 2023/09/02
- PDFにサインをしてEmailで返送。2023年時点では書類の郵送の必要はなく、メールで完結した。
- （印刷、スキャンが手間でタスクを積みっぱなしにしていたら、事務局からリマインドをいただいてしまった
- 2023/09/20
- Assignmentのプロセスが完了の連絡受信
- 2023/09/26
- 2023/10/05
- yantar92さんからレビューをもらう
line-number-at-posは使わないほうがいいのじゃないかという指摘。納得すぎる
- 2023/10/08
- レビューでの指摘を対応し再提出
- 2023/11/08
不安な日々
メールを送る機会が少なくなっていて、メールの送信者を日本語の本名で送ってしまった。 またGmailでパッチファイルを添付したのだが、他の人が読めるファイル形式になっているのか、返信をもらうまで不安を抱えていた。
この辺りは、送信者名はAscii文字列でなければないだろうし、何か失敗があってもコントリビュートは恥をかきながら覚えようということで開き直ったりもしていた。
レビュワーへの感謝
レビュワーのyantar92さんからは、コミットメッセージの書き方、コードフォーマットの作法、効率的なアルゴリズムなどを丁寧に指摘していただいた。パッチに対してレビュワー自身がコードを改変したほうが労力は少なかっただろうに、一見さんの私に対して丁寧な教育をしてもらえたのがとても嬉しかった。業務、OSSを問わず、レビューをする際にはこの経験を生かしたいと思う。
まとめ
org-modeへの初のコントリビュートについてまとめた。 メーリスへの投稿から数週間レスがない時期などは、もしかして忘れられてないだろうかと不安になることもあったが、最終的にマージされた時の嬉しさは格別だった。
マージについてお礼を言った際に返ってきた
Music to our ears - thanks to you !
というお洒落な返事も嬉しさを大きくさせてもらった。
世界で使われているソフトウエアに貢献できるのであれば、せっかくならば英語ももう少し勉強してみようかなと思ったものだった。
参考文献
著作権譲渡の手続きやメーリングリストへの投稿方法は下記の記事が参考になった。
