kiwanami / emacs-window-manager

Customizable window manager for emacs
230 stars 28 forks source link

Feature request: make e2wm:pst-update-windows extendable #18

Closed tkf closed 12 years ago

tkf commented 12 years ago

e2wm:pst-update-windows 相当の関数を :update にしたものをすべてのパースペクティブの base class にしてはどうでしょうか。 Monky パースペクティブの問題点があるのですが、それをスマートに解決するのに役立ちます。

Monky パースペクティブを開くと各プラグインの更新に時間がかかり、かなりまったりな動作になってしまいます。 Monky (Magit にも) monky-with-refresh なるマクロがあってこのマクロ内では画面の更新をせず、一番最後に一回だけ更新をおこなう、みたいなことが出来ます。これを e2wm で試すには、 e2wm:pst-update-windows を以下のように変更すればいけます。

(defun e2wm:pst-update-windows (&optional rebuild-windows)
  ;;全ウインドウを更新する。rebuild-windowがnon-nilであればウインドウの再構築を行う。
  (e2wm:message "#PST-UPDATE")
  (e2wm:with-advice
   (let* ((instance (e2wm:pst-get-instance))
          (wm (e2wm:$pst-wm instance)))
     ;;(e2wm:debug-windows (e2wm:pst-get-wm))
     (unless rebuild-windows
       (wlf:wset-fix-windows wm))
     (when (or rebuild-windows 
               (not (wlf:wset-live-p wm)))
       (e2wm:message "  #PST-UPDATE > REBUILD")
       (wlf:refresh wm)
       (e2wm:aif (e2wm:$pst-main instance)
           (wlf:select wm it)))
     (monky-with-refresh                   ; ← これを追加
       ;;パースペクティブ固有の処理
       (e2wm:pst-method-call e2wm:$pst-class-update instance wm)
       ;;プラグイン更新実行
       (e2wm:plugin-exec-update (selected-frame) wm))
     )) t)

これをもうちょっとまともに解決するためには、 e2wm:pst-update-windows 相当の関数を :update にしたものをすべてのパースペクティブの base class にして、すべてのパースペクティブをその base class から派生させれば良いと思います。そうすれば、こんな感じで monky-with-refresh が使えるはずです:

(defun e2wm:dp-monky-update (wm)
  (monky-with-refresh
    (e2wm:$pst-class-super wm)))
kiwanami commented 12 years ago

確かに、 :update はほとんど使ってないので、提案していただいた内容でも悪くないかもと思いました。

そもそも、この「各ウインドウへの更新通知」のメカニズムはシンプルすぎて、「更新の理由が分からないため各プラグインが更新すべきかしないべきかを判断することが出来ずに、毎回描画し直すことしかできない」ことが問題かなと思っています。最初に作った時からここは改善したいと思っていて、concurrent.elのcc:channelが完成したら、このあたりの通知を非同期イベントにしてしまおうと思っていました。

そうすると、更新イベントをパースペクティブ自体がsubscribeして各プラグインバッファをまとめて更新し、各プラグインは更新通知が来てもバッファを表示するだけ、ということが出来るかなと思っています。

ただ、この非同期イベント修正はちょっと時間がかかりそうなので、もしtkfさんの方でこちらの修正がすぐ出来て pull request いただけたら取り込みたいと思いますが、お願いしてもよろしいでしょうか?

tkf commented 12 years ago

時間あるときにやってみます。パースペクティブのprefixは e2wm:dp-base- で良いですか?

非同期イベント化は楽しみです! monky みたいにプロセスをひとつしか持てないようなやつが各 plugin をバラバラに更新した時にどうなるかがちょっと怖いですが。

tkf commented 12 years ago

変数名とか直したほうがよければ、またpull request入れます。

最後のコミット(0fa0549ceaadc046502f3d86f4e61b97decaa132)はもし要らなければ外しても機能は変わらないはずです。

tkf commented 12 years ago

最後のコミット修正しました: a9bf7f06e5f67d215dce618fb247fd204f4bf3fe

kiwanami commented 12 years ago

すごい!早い! ありがとうございます! すぐ確認して取り込みます。

kiwanami commented 12 years ago

問題ないようでしたので取り込みました。 ウインドウ4列とか自分よりもかなり使いこなしてらっしゃるようで、有難い限りです。

tkf commented 12 years ago

pull ありがとうございます!意外と単純な変更で済んだので何か見落としてないか心配でした。

tkf commented 12 years ago

そういえば e2wm:dp-doc-update 内で super の関数よんでないですが、 doc perspective では plugin 使ってないので今のところ問題無いはずです。ただ、将来的に base perspective を拡張するかもしれないことを考えると super よんだほうが良いかもです。

tkf commented 12 years ago

20 ですが、後方互換性のない変更になっているので、自分でパースペクティブ定義しているユーザーが不便に思うかもしれません。 最近 @myuhe さんがリリースした e2wm-R.el が被害にあいそうです。

解決法として #22 を書いてみました。ただ、この解決法を適用すると base に依存しないクラスをかきたい時に自分で抽象クラスを作ってそれを継承するとうまどろっこしいことをすることになります。個人的にはこのパッチ適用しないほうが良いんじゃないかと思うんですが、互換性を守る重要性が高ければpullしてください。ご迷惑をおかけします。。。

あと、意外と複雑な関数になってしまったので、精査お願いします。

tkf commented 12 years ago

23 は2つ前のコメントの e2wm:dp-doc-update に関する fix などで、後方互換性に関するやつとは無関係です。

kiwanami commented 12 years ago

ちょっと調べてみます

myuhe commented 12 years ago

なんだか、気を使わせてしまってすみません。。。 後方互換性はなくなりますが、僕もtkfさんが言うように現在のままで良いんじゃないかと思います 現在のバージョンに合わせた変更もそこまで大変なものでもなさそうですし。

kiwanami commented 12 years ago

23を取り込まない方針で考えています。

その際、問題になる継承の階層の制限をなくしてみました。 https://github.com/kiwanami/emacs-window-manager/commit/70ac168d5941d3cddf8123e98c2107fede0efd2c

手元では動作確認できていますが、内部用の関数である 'e2wm:$pst-get-prop' と 'e2wm:method-call' の関数のインタフェースを変えたので、これらを使っている場合はもしかしたらみなさんのパースペクティブ定義で問題が出るかもしれません。 よろしければご確認をよろしくお願いします。

myuhe commented 12 years ago

'e2wm:$pst-get-prop' と 'e2wm:method-call' を含んだものではありませんが自作のパースペクティブに新しいbase class定義に関連する修正を加えたところ、問題なく動作しているようです。 いちお報告まで

kiwanami commented 12 years ago

ご確認ありがとうございます!