-
Notifications
You must be signed in to change notification settings - Fork 163
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] マクロの Python 対応 #1327
[WIP] マクロの Python 対応 #1327
Conversation
❌ Build sakura 1.0.2862 failed (commit f50e760e0a by @beru) |
❌ Build sakura 1.0.2863 failed (commit 3858512f3f by @beru) |
❌ Build sakura 1.0.2864 failed (commit 27d554df39 by @beru) |
Pythonマクロの導入提案には賛成します。 ひとまず、このPRでWin32 Debugのビルドが失敗している原因は調べたほうがよいのかな・・・ |
なお、参考情報ですが、サクラエディタは ActiveScripting に対応しているので、EmEditor同様にActiveScriptingに対応したスクリプトエンジンをインストールしさえすればソースコード変更なしにpythonスクリプトを使うことができるはずです。 http://emeditor.web.fc2.com/EmEditor_Macro_ActiveScript.html 標準のpythonじゃなくてactive pythonを入れないといかんのがネックですが・・・。 |
お、これ知らなかったです。サクラエディタは ActiveScripting に対応しているっていうのはソースコードだとどこら辺が相当するのか調べてみます。 ActiveStateは懐かしいですね。結構昔にWindows上で Perl を動かしたい時に ActivePerl を使ってました。 https://www.activestate.com/products/python/downloads/ を見ると、3.7 までしか対応していないのであんまりアクティブに最新版に追従していないのが残念なとこですね。 |
|
Python/C API の知識はレビューに要ると思います。自分は今回初めて使ったんですが不明な事が多くて色々と調べました。
Issue 立てて色々とコメントを交わしてもそれだけではこのPRのコードがレビュー出来るようになるとは思えませんが、Python導入の方針とかを落ち着いて議論して決めたいのであれば有用だと思います。 自分が今の実装で気にしている事ですが、暗黙的なリンクにしているのでビルド環境に Python のヘッダファイルや lib ファイルが存在しないとビルドに失敗します。この実装方法だと、Pythonを使わないからインストールはしていないという人がサクラエディタをビルドしようとした時にビルドがコケてしまうので問題になりそうです。
現状 AppVeyor で Win32 のビルドは成功していて x64 のビルドが失敗しています。 https://ci.appveyor.com/project/sakuraeditor/sakura/builds/33769185 64bit の Python が https://www.appveyor.com/docs/windows-images-software/#python ただ AppVeyor で仮にビルド出来たとしても、Azure Pipelines でのビルドに失敗している問題は残ります。 |
あと今の実装で気にしている他の事ですが、利用する Python のバージョンが 3.8 固定になっているというのがあります。 limited API に限定して使用する事で複数のバージョンのPythonに対応出来そうですが、使えるAPIが制限されている分実装に躓く事が多いと思います。 |
実装はここです。 sakura/sakura_core/macro/CWSH.cpp Lines 245 to 246 in 74a0675
AEngineには L"JScript" みたいなスクリプトエンジン名を入れる仕様です。ここにシステムにインストールした python のスクリプトエンジンの識別名を入れることにより、次のコードでスクリプトエンジンのインスタンスが生成されます。 sakura/sakura_core/macro/CWSH.cpp Lines 255 to 256 in 74a0675
なんとなく嫌、は同意 😃 |
公式版のPythonのインストール先はレジストリから取得することが出来ます。 なお、VimもPythonインターフェースを持っていますが、使用するPythonのバージョンはコンパイル時には選択可能ですが、実行時には変更できません。 |
サクラエディタには |
今はどうやってスクリプトエンジン名を取得しているのかコードを調べてみました。
試してませんが ActivePython をインストール環境した環境では今の実装でもPythonスクリプトをマクロとして使えるって事なんでしょうか?確認しようと ActivePython をダウンロードしようとしたらアカウント作成を求められたので諦めました。 なんとなく嫌なのは本家の https://www.python.org/ でWindows版のインストーラー配布してるからわざわざ他のを入れる必要性が無いというのもありますね。。パッケージ管理も別になってしまうし…。 |
ぼくも最近は試していません。 GitHubに移行するよりも前に、Activeperlで実験したことはあって、そのときは拡張子plに対するレジストリエントリでperlスクリプトを実行できた記憶があります。pythonスクリプトの拡張子はpyなので、pyでいけるんだと思います。 |
お、情報ありがとうございます。きっと64bit版のWindowsでのパスですね。恐らく32bit版のWindowsを使ってる人はもう殆どいないし、32bit版のWindowsでビルドする事を考慮する必要は無いかもしれませんね。 ただその値を何かで読み取ってファイルに書き込んだりするのは手間ですね。CMake だと find_package で楽に探せますがその為に色々と実装をしているようです。 https://gitlab.kitware.com/cmake/cmake/-/blob/master/Modules/FindPython/Support.cmake
複数バージョンのPythonがインストールされている環境もあるので、その場合にビルド時、もしくは実行時にどのバージョンのPythonを使うのかを指定出来るようにする必要があるか?というのは開発時の決め事の1つですね。 自分は特定のバージョンの Python のDLLファイル、例えば |
このPRのやり方で拡張子 py に対応する新規コードを入れると、ActivePython をインストールしている環境では ActivePython が使われる事になりそうです。先に |
動的に Python/C API を使う為に必要な記述が色々多くて大変そうだなぁ…って思ってたんですが下記のコードではそれやってるようです。 https://github.com/rstudio/reticulate/blob/master/src/libpython.h |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
このPRの問題点は3つあります。
考えられる解決方法はこんな感じ。
|
1 に関してですが、現状のようにDLLの暗黙的リンクをするにはビルド環境に Python がインストールされている必要があります。AppVeyor の場合は決まったパスにPythonがインストールされている事は分かりました。が、Azure pipelines についてはまだ未調査です。ユーザーのビルド環境ではどのパスにインストールされているか決め打ちに出来ない事を考えると、確かに CMake を使う対策が良さそうに思えます。 しかし https://github.com/sakura-editor/sakura#build-requirements にはビルドに必要なものの中に CMake や Python というのは書かれていません。つまりそれらが存在しない環境でビルドが失敗するようなPRはそもそもNGという前提があります。という事で CMake を使うのはどうかなと思います。またPythonについてもインストールされているとは限らないのでDLLの暗黙的リンクを行うプロジェクト設定には出来ないのではないかと思います。 そうすると
コードレビューをする場合は書かれているコードの内容を理解する必要があるので、CPython の Python/C API を ただPRのレビューは必ずしもコードレビューが必要とは自分は思わないです。せっかくコードが見れるのだからコードレビューした方が良い事は確かなんですが、ビルド生成物を操作してPRの目的が満たせているかや不具合が無いかを確認するやり方でも良いんじゃないかとは思います。とはいっても現実的にはほぼコードを書いている人達しかPRのレビューをしない感じですが…。
Python 2のサポートはもう切れているという事で対応しない方が良いと思います。 Python 3.8 しか今のところ対応する気はないですが、それは単に自分が複数バージョン(例えば Python 3.7 と Python 3.8)のPythonをサポートするやり方がよくわからないからです。対応する必要があるかどうかというのは要件の話になりますが、自分は対応出来た方が良いけれど別に対応しなくても良いんじゃないかな派です。
CMake がインストールされているか確認して、インストールしていない環境でビルドがこけないようにするならそれも有りだと思います。 また、C++コード側では、 #if __has_include(<Python.h>)
#endif とかして書けば、ビルド環境に Python がインストールされていなかったり、Python の include フォルダへのパスを通せていなくても、サクラエディタのビルドがこけないように出来るかもしれません。
いくつかの点を決めた方が良いと思います。
|
必要なのであればPRの中で追記すればいいだけだと思います。ユーザー環境で必須になる訳ではないですし。
明示的リンクをするにしても、将来的なPythonのバージョンアップを考えると vimの場合は暗黙的・明示的リンクの両方に対応していますが、
ですね。Python 2に先がないことは10年以上前からアナウンスされてましたし、新規開発では一切考慮に値しないでしょう。
ビルド時にバージョン判別するのであれば |
特定バージョンのpythonへの依存を半強制的に解決させる手段として NuGet パッケージが使えるんじゃないかと思いました・・・。 https://www.nuget.org/packages/python/3.8.3 ← 標準verはx64専用です。 |
コード上の気になることをちょっとコメントしてみます。 それとは別に考慮済みっぽくて釈迦に説法かもしれませんが、ExecExternalMacroマクロ関数がありまして「マクロを10回実行するマクロ」とかも書けることになっています。
wcscmpになっています。Windows版のパイソンの拡張子は大文字、小文字区別するんでしょうか。
ちょっと某所でも話題だった、サクラエディタの行データなどはNUL文字を含むので、WSHなどはなるべく文字列長を保持してC文字列を避けています。 |
放置期間が1年を超えました。必要であれば「再開するときに」再オープンしてください。 |
再オープン出来ないので別のMRを作成します。 |
PR の目的
サクラエディタのマクロを Python で書けるようにする事が目的です。
カテゴリ
PR の背景
現状は WSH(JScript, VBScript)マクロ と PPAマクロが使えます。PPAマクロは 32bit 版のみ対応です。
PR のメリット
PR のデメリット (トレードオフとかあれば)
Python38.dll
を遅延ロードするように設定したので、実行環境で Python 3.8 がインストールされていなくてもサクラエディタの起動は行えると思います。C:\Python38
固定になっています。仕様・動作説明
メニューの
ツール
>名前を指定してマクロ実行
を選ぶとファイルダイアログが表示されますが、そこで拡張子が*.py
の Python ファイルを選べるようにしました。Python スクリプト側では
import SakuraEditor
と記述してそのモジュールの関数を使ってサクラエディタのコマンドやマクロ関数を呼び出す事が出来ます。テスト内容
開発時には上記のようなPythonスクリプトのファイルを指定して実行確認を行いました。
PR の影響範囲
マクロ関連
関連 issue, PR
参考資料
https://docs.python.org/3/extending/embedding.html