Skip to content
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

マクロの Python 対応 #1866

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

マクロの Python 対応 #1866

wants to merge 18 commits into from

Conversation

beru
Copy link
Contributor

@beru beru commented Oct 2, 2022

PR の目的

サクラエディタのマクロを Python で書けるようにする事が目的です。

カテゴリ

  • 機能追加

PR の背景

サクラエディタは WSH(JScript, VBScript)マクロ と PPAマクロが使えます。PPAマクロは 32bit 版のみ対応です。
最近Pythonが広く使われるようになっているので、それに対応したくなりました。

このPRでは LoadLibrary したDLLから GetProcAddress を使って明示的なリンクでシンボルを探して使用する事で、ビルド環境にPythonが無くてもビルド出来るようにしています。

PR のメリット

  • Pythonでマクロが書けるようになります。
  • Pythonのエコシステムを間接的に使う事が出来ます。

PR のデメリット (トレードオフとかあれば)

仕様・動作説明

メニューの ツール > 名前を指定してマクロ実行 を選ぶとファイルダイアログが表示されますが、そこで拡張子が *.py の Python ファイルを選べるようにしました。

Python スクリプト側では import SakuraEditor と記述してそのモジュールの関数を使ってサクラエディタのコマンドやマクロ関数を呼び出す事が出来ます。

テスト内容

from time import time,ctime
import SakuraEditor

#SakuraEditor.FileNew()
#SakuraEditor.FileOpen("", 0, 0, "C:\\")
#SakuraEditor.GetFilename()
clipboard = SakuraEditor.GetClipboard()
SakuraEditor.AddTail("clipboard : " + clipboard)

print('Today is', ctime(time()))

開発時には上記のようなPythonスクリプトのファイルを指定して実行確認を行いました。

PR の影響範囲

マクロ関連

関連 issue, PR

#1327

参考資料

https://docs.python.org/3/extending/embedding.html

@beru beru added the enhancement ■機能追加 label Oct 2, 2022
@AppVeyorBot
Copy link

Build sakura 1.0.4182 completed (commit 175294c129 by @beru)

Copy link
Contributor

@berryzplus berryzplus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

以前コメントした通りです。
#1327 (comment)

ビルドできない状態はマズいので、エラーの解消は行って欲しいです。

何をもって「よし」とするかは他の方に任せます。

for (size_t i = 0; i < _countof(symbols); ++i) {
auto& s = symbols[i];
auto sym = ::GetProcAddress(s_hModule, s.name);
*(void**)s.ptr = sym;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

キャストでエラーが出ています。

Suggested change
*(void**)s.ptr = sym;
*(FARPROC*)s.ptr = sym;

Copy link
Contributor Author

@beru beru Oct 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MinGW のビルドでエラーが起きていますね。

https://dev.azure.com/sakuraeditor/8a4927d0-fbf2-4d46-9d36-17af9073ab33/_apis/build/builds/3421/logs/221

2022-10-02T16:38:46.0385436Z D:/a/1/s/sakura_core/macro/CPythonMacroManager.cpp: In member function 'virtual bool CPythonMacroManager::ExecKeyMacro(CEditView*, int) const':
2022-10-02T16:38:46.0717050Z D:/a/1/s/sakura_core/macro/CPythonMacroManager.cpp:933:34: error: invalid conversion from 'long long int (*)()' to 'void*' [-fpermissive]
2022-10-02T16:38:46.1208526Z   933 |                 *(void**)s.ptr = sym;
2022-10-02T16:38:46.1213459Z       |                                  ^~~
2022-10-02T16:38:46.1242539Z       |                                  |
2022-10-02T16:38:46.1277863Z       |                                  long long int (*)()

うーん、右辺の関数ポインタをキャスト無しで void* に暗黙的に 変換 代入 するのは無しだよって事かな。。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ff2b219 で右辺を明示的に void* にキャストするようにコードを変更しました。

今使っている環境にMinGWを入れていないので(まぁ入れればいいんですが…。)、ビルドエラーが取れるかはCIのログで確認したいと思います…。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MinGWのビルドエラーが解消されました。

::VariantInit(&vtArgs[i]);
if (varType == VT_BSTR) {
const char* str = PyUnicode_AsUTF8(arg);
SysString S(str, (int)strlen(str));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strlen(str)セキュリティ問題を含んでいます。
strNUL終端されていない場合、アクセスしてはいけない領域を読み込んでしまいます。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PyUnicode_AsUTF8AndSize を使って取得したサイズを使うようにすれば良さそうですね。

https://docs.python.org/3/c-api/unicode.html#c.PyUnicode_AsUTF8AndSize

Copy link
Contributor Author

@beru beru Oct 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ff2b219PyUnicode_AsUTF8AndSize を使うように変更しました。

この関数は Python 3.3 で追加されたようで 3.10 から Stable API になったようです。

自分は Python 3.9.6 をインストールしていて python3.dll にPATHが通っている環境でしか動作確認していないので、環境によっては正しく動作しないかもしれません。

k-takataさんが #1327 (comment) してくれたように公式版のPythonのインストール先はレジストリから取れるようなので、ユーザーに優しくするならPATHが通っていない環境でも実行できるようにするべきかもしれません。

…rable strlen can be avoided

explicitly cast function pointer to void* before copying it to void* type variable
::VariantInit(&vtArgs[i]);
if (varType == VT_BSTR) {
Py_ssize_t sz = 0;
const char* str = PyUnicode_AsUTF8AndSize(arg, &sz);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

エラー時に NULL が返ると記載されていますが、それの考慮が出来ていません。

https://docs.python.org/3/c-api/unicode.html#c.PyUnicode_AsUTF8AndSize

@AppVeyorBot
Copy link

Build sakura 1.0.4183 failed (commit 4bed86e126 by @beru)

{
static HMODULE s_hModule;
if (!s_hModule) {
s_hModule = LoadLibrary(L"python3.dll");
Copy link
Contributor Author

@beru beru Oct 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

codefactor が Possible use of current directory (CWE-829) の問題報告をしています。
https://www.codefactor.io/repository/github/sakura-editor/sakura/pull/1866

DLLプリロード攻撃に該当するようです。

https://shirouzu.jp/tech/dllpreload_attack#%E5%BE%93%E6%9D%A5%E3%81%AE%E5%AF%BE%E7%AD%96%E3%81%A7%E8%80%83%E6%85%AE%E3%81%8C%E5%BF%85%E8%A6%81%E3%81%AA%E7%82%B9

サクラエディタでは LoadLibraryExedir 関数が用意されていて、これはDLLインジェクション対策としてカレントディレクトリをEXEのディレクトリに変えてから LoadLibrary APIを呼び出すようにしています。

サクラエディタがDLLインジェクション対策をしないとどういう問題が起きうるのかをあまり分かっていませんが、ユーザー環境に不正な python3.dll ファイルが置かれている場所からサクラエディタを起動されると、不正なコードが実行される可能性が生じるので良くないという事のような気がします。

そうなるとやはり #1327 (comment) に書かれているように公式版のPythonのインストール先はレジストリから取ったり、サクラエディタの設定情報としてPythonのDLLをどこからLoadするかを持つようにする必要が出てきそうです。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

12ba7b8LoadLibrary の代わりに LoadLibraryExedir を使うように変更しました。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoadLibrary関数が失敗した場合にエラーメッセージが表示されないので、Pythonスクリプトを実行したのかしていないのか判断がしにくい事に気づきました。例えば、64bit版のPythonをインストールした環境でWin32のサクラエディタを起動しても、python3.dll の読み込みが出来ません。

という事で 5bc588b で失敗時にエラーメッセージを表示する処理を追加しました。

if (varType == VT_BSTR) {
Py_ssize_t sz = 0;
const char* str = PyUnicode_AsUTF8AndSize(arg, &sz);
SysString S(str, (int)sz);
Copy link
Contributor

@usagisita usagisita Oct 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strはchar*ですがUTF-8、SysStringのchar*版はANSI/CP932ではありませんか。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

指摘されるまで意識出来てませんでしたが、確かにそうですね(地域設定の Beta: Use Unicode UTF-8 for worldwide language support を指定した時の挙動はよくわからない)。

SysString(const char *S, int L)
{
wchar_t *buf = new wchar_t[L + 1];
int L2 = ::MultiByteToWideChar(CP_ACP, 0, S, L, buf, L);
Data = ::SysAllocStringLen(buf, L2);
delete[] buf;
}

MultiByteToWideCharCP_UTF8 を指定して UTF-8 から UTF-16LE に変換出来るようですが、そのWindowsAPIを使うべきか、それとも内部実装の CUtf8::UTF8ToUnicode を使うべきかはちょっと判断が付きません。後者はいったん CNativeW 型と CNativeA 型を使う必要が出そうですね。

Copy link
Contributor Author

@beru beru Oct 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handleCommand の方だと PyUnicode_AsWideCharString を使っていますね。それを使う方がシンプルに済みそうです。というか戻り値を返すかどうかという違いなので引数の処理は共通化出来そう…。。

追記 : std::wstringVARIANT という違いが存在

std::vector<PyMethodDef> g_commandDescs;
std::vector<std::string> g_functionNames;
std::vector<PyMethodDef> g_functionDescs;
CEditView* g_pEditView;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#1327 (comment) で usagisita さんから指摘されている点として、グローバル変数 g_pEditView を使ってるけど再入可能性を考慮していないっぽいよという事ですが、はい考慮していませんでした。。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bc086a4 でグローバル変数 g_pEditView を使うのを止めて、&GetEditWnd().GetActiveView()CEditView* を取るようにしました。


g_pEditView = EditView;

PyObject* pCode = Py_CompileString(m_str.c_str(), to_achar(m_pszPath), Py_file_input);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m_strはUTF-8だと思いますが、to_achar(m_pszPath)はCP932でいいんでしょうか。
Pythonのドキュメントを軽く見ただけでは、これがどのように解釈されるのか分からないので、なんとも言えませんが。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

良く分からなかったのでPythonのソースコードを読んでみました。

ドキュメントに書かれているように Py_CompileStringPy_CompileStringExFlags のラッパーで

https://github.com/python/cpython/blob/571e23d99157ed7ad67ca2334a396fc9ddbe07ec/Python/pythonrun.c#L1981-L1985

Py_CompileStringExFlagsfilename 引数のコメントに decoded from the filesystem encoding と書かれていました。

https://github.com/python/cpython/blob/571e23d99157ed7ad67ca2334a396fc9ddbe07ec/Include/cpython/pythonrun.h#L57-L62

実装では PyUnicode_DecodeFSDefault を呼んでdecodeしているようです。

https://github.com/python/cpython/blob/571e23d99157ed7ad67ca2334a396fc9ddbe07ec/Python/pythonrun.c#L1807-L1818

https://docs.python.org/ja/3/c-api/unicode.html#c.PyUnicode_DecodeFSDefault

https://docs.python.org/ja/3/glossary.html#term-filesystem-encoding-and-error-handler

https://docs.python.org/ja/3/glossary.html#term-locale-encoding

という事で多分今のままでも良いのかもしれませんが、テストはしておらず良く分かりません。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

少し前のバージョンからWindowsでもfilesystem encodingはUTF-8になったはずです。

>>> import sys; print(sys.getfilesystemencoding())
utf-8

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

情報ありがとうございます。

自分の環境でも同じ結果になる事が確認出来ました。

image

という事で CPythonMacroManager::LoadKeyMacro メソッドで WideCharToMultiByte(CP_UTF8 を使って事前に UTF-8 文字列を作るようにします。 (m_pszPath メンバ変数は std::string m_path とかに変えて)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4cc6343 でUTF-8に変換した文字列を渡すように変更しました。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3.6 より前のバージョンへの対応はとりあえず考えない事にします。

引数の filename はエラーメッセージとかに使われるようです。Python実行のエラーメッセージの取得は PyErr_Fetch を使う事で出来ました。https://stackoverflow.com/a/1418703

しかしサクラエディタのマクロ実行のエラーを出力するウィンドウが無さそうなのでどこにエラーメッセージを出すか悩みます。WSHマクロではどうしているかを調べてみようと思います。

Copy link
Contributor Author

@beru beru Oct 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

サンプルの tools/macro/CopyDirPath/CopyDirPath.js ファイルをコピーしてからエラーが起きるように変更して(CopyDirPath.js.txt (拡張子偽装))実行してみました。すると下記のメッセージボックスが出ました。

image

どこで出ているかを確認してみたところ、CWSHMacroManager::ExecKeyMacro から呼び出される CWSHClient::Execute から呼び出す Parser->ParseScriptText の呼び出しのところでエラー用のコールバック処理が呼び出されてメッセージボックスが表示されていることが分かりました。

image

という事でエラー時には同じようにメッセージボックスでエラー内容を表示してあげる必要がありそうです。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

15a80d2 でエラー時にメッセージ表示する処理を追加しました。

// static
CMacroManagerBase* CPythonMacroManager::Creator(const WCHAR* FileExt)
{
if (wcscmp( FileExt, L"py" ) == 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

大文字じゃだめなんでしたっけ。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

区別しないように実装を更新するべきですね。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_wcsicmp 関数を使うように更新しました。


BOOL CPythonMacroManager::LoadKeyMacro(HINSTANCE hInstance, const WCHAR* pszPath)
{
FILE* f = _wfopen(pszPath, L"rb");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

一応メモとして、ファイルオープンのエラーチェックぅは欲しいですね。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

そのエラーチェックは必要ですね。修正します。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ceb8e76 でエラーチェックを追加しました。

@AppVeyorBot
Copy link

Build sakura 1.0.4184 completed (commit 9f6b5024cf by @beru)

@AppVeyorBot
Copy link

Build sakura 1.0.4185 completed (commit 2496668abb by @beru)

wide2utf8(m_strPath, pszPath);
long sz = _filelength(_fileno(f));
m_strMacro.resize(sz);
fread(&m_strMacro[0], 1, sz, f);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

やってみないでいうのもなんなのですが、BOMがあっても動くのでしょうか。
あと他のタイプのマクロは規定がSJISでUTF-8はBOM付のときだけ有効ですね。
他の実装では読み込み時にBOMがデータから削除されていたと思います。
もっとも2022年の現代からするとPython実装では規定でUTF-8で構わないと思います。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BOMの事を考えていなかったですが確認が必要ですね。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

動作確認してみたところ、BOMがあると動きませんでした。
なのでUTF-8 BOM付きのpyファイルを読んでも動くように対策を入れようと思います。

なお本来は色々な文字エンコーディングのファイルを読めるテキストエディタなので、仮にファイル読み込み処理をうまく再利用出来るならそうするのが良いと思いますが、(多分出来なそうなので)限定した文字エンコーディングのファイルのみに対応という事になると思います。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3f95404 で対策しました。

@AppVeyorBot
Copy link

Build sakura 1.0.4186 completed (commit 2c648564d8 by @beru)

@AppVeyorBot
Copy link

Build sakura 1.0.4187 completed (commit ee22350f6b by @beru)

@berryzplus berryzplus dismissed their stale review October 11, 2022 09:27

ビルドエラーが解消したので。

@AppVeyorBot
Copy link

Build sakura 1.0.4188 completed (commit c73651a02d by @beru)

@sonarcloud
Copy link

sonarcloud bot commented Oct 11, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell C 311 Code Smells

3.2% 3.2% Coverage
0.0% 0.0% Duplication

@AppVeyorBot
Copy link

Build sakura 1.0.4189 completed (commit eaad38d484 by @beru)

@AppVeyorBot
Copy link

Build sakura 1.0.4204 completed (commit bae93df920 by @beru)

@AppVeyorBot
Copy link

Build sakura 1.0.4205 completed (commit a7b366d02a by @beru)

@berryzplus
Copy link
Contributor

仕様の疑問点です。

import SakuraEditor

#SakuraEditor.FileNew()
  1. import SakuraEditorの記述は必須ですか?
    javascriptとかだと不要なので、合わせたほうが良い気がしました。
  2. #SakuraEditor#は必須ですか?
    pythonに詳しくないからか、見たことない書き方でした。

@berryzplus
Copy link
Contributor

CIのチェックに関して。

  1. CodeFactorの指摘(関数の複雑度高過ぎ×2件)
    対応無理なのでスルーでよい認識。
  2. SonarCloudの指摘(コードカバレッジ8割未達、CodeSmell多数)
    カバレッジ8割クリアは厳しいが、修正部分に対して3%はあまりに低いような。
    CodeSmellはスルーしてよい認識。

結局、何をもってOKにしますか?になりそう。

@beru
Copy link
Contributor Author

beru commented Dec 19, 2022

@berryzplus さん

仕様の疑問点です。

import SakuraEditor

#SakuraEditor.FileNew()
  1. import SakuraEditorの記述は必須ですか?
    javascriptとかだと不要なので、合わせたほうが良い気がしました。

CPythonMacroManager::ExecKeyMacro の実装に手を入れる事で不要にすることは出来るかもしれません。

  1. #SakuraEditor#は必須ですか?
    pythonに詳しくないからか、見たことない書き方でした。

Python では行コメントに # 文字を使います。shell script といっしょですね。

@beru
Copy link
Contributor Author

beru commented Dec 19, 2022

@berryzplus さん

結局、何をもってOKにしますか?になりそう。

おおまかには、Pythonスクリプトで書いたサクラエディタのマクロを実行できる環境が整った段階でOKじゃないかなと思います。

動作確認に関しては、どのバージョンのPythonから動作するか、マクロの関数とコマンドが一通りの問題無く動くのかの確認等をした方が良さそうですが、これはPR作成者がまず行うべきですね。

ユーザーが使う際にサクラエディタのインストールだけでなく追加でPythonのインストールも必要になるので、少し導入に敷居があるかもしれないですね。

@berryzplus
Copy link
Contributor

  1. import SakuraEditorの記述は必須ですか?
    javascriptとかだと不要なので、合わせたほうが良い気がしました。

CPythonMacroManager::ExecKeyMacro の実装に手を入れる事で不要にすることは出来るかもしれません。

これは「どっちがいい?」と聞いてます。

「やりたい」「やりたくない」「どっちでもいい」があって、
「実装する」「実装しない」「そのうち実装するかもしれない」があると思います。

かなり面倒だと思うので「やらん」でOKです。
pipの話を持ち出すとややこしいんですが、将来的にやる想定なら「やらん」が正しい気がします。

2. #SakuraEditor#は必須ですか?
pythonに詳しくないからか、見たことない書き方でした。

Python では行コメントに # 文字を使います。shell script といっしょですね。

これはコードを読み違えてたのでスルーしてもらって大丈夫です。

結局、何をもってOKにしますか?になりそう。

おおまかには、Pythonスクリプトで書いたサクラエディタのマクロを実行できる環境が整った段階でOKじゃないかなと思います。

動作確認に関しては、どのバージョンのPythonから動作するか、マクロの関数とコマンドが一通りの問題無く動くのかの確認等をした方が良さそうですが、これはPR作成者がまず行うべきですね。

「マクロの関数とコマンドが一通り問題なく動く」を確認する方法をどうしましょう。

python + appium でRPAできないか検討してみてますが、
いったんは手動確認するしかないかなぁと思ってます。
ターゲットのpythonバージョンは最新(か、公式でサポートされている最古のバージョン)でよいと思います。

ユーザーが使う際にサクラエディタのインストールだけでなく追加でPythonのインストールも必要になるので、少し導入に敷居があるかもしれないですね。

CIでのテスト、開発時に行うテストに限定して考えれば、最新のpythonインタープリタがインストールされているを前提して進めて良い気がします。python使わない人がpythonマクロ使うとも思えないのでインストーラーへの組み込みは後回しでよいと思います。

@beru
Copy link
Contributor Author

beru commented Dec 20, 2022

これは「どっちがいい?」と聞いてます。

「やりたい」「やりたくない」「どっちでもいい」があって、 「実装する」「実装しない」「そのうち実装するかもしれない」があると思います。

かなり面倒だと思うので「やらん」でOKです。 pipの話を持ち出すとややこしいんですが、将来的にやる想定なら「やらん」が正しい気がします。

私個人の意見としては「どっちでもいい」です。そういえばマクロのPython対応をするとなるとヘルプの更新も必要になりそうですね。

「マクロの関数とコマンドが一通り問題なく動く」を確認する方法をどうしましょう。

python + appium でRPAできないか検討してみてますが、 いったんは手動確認するしかないかなぁと思ってます。 ターゲットのpythonバージョンは最新(か、公式でサポートされている最古のバージョン)でよいと思います。

手動確認を考えていました。テスト結果のEvidenceが自己申告になりますが…。

Python + Appium でGUI自動操作して確認は出来ると思いますが、それを使ったテストケースの用意とパスがPRのマージに必要、という条件を設定してほしくはないです。個人的には、新しいハードルを追加で用意してそれをクリアしないと駄目、というように決めるのはPRがマージされるペースが落ちてしまうのでやってほしくないです。

CIでのテスト、開発時に行うテストに限定して考えれば、最新のpythonインタープリタがインストールされているを前提して進めて良い気がします。python使わない人がpythonマクロ使うとも思えないのでインストーラーへの組み込みは後回しでよいと思います。

CIでのテストに関しては、CIのサービスが用意している最新のPythonを使うので良さそうですね。

開発時に行うテストに関してはPython必須にはしたくないです。

サクラエディタのインストーラーにPythonランタイムを組み込むことは考えていません。Portable Winpythonを使う事で可能かもしれませんが、容量が無駄に増えてしまうのは好ましくありません。

@berryzplus
Copy link
Contributor

私個人の意見としては「どっちでもいい」です。

はい。では「やらない」で。
当面それで問題ないように思います。

そういえばマクロのPython対応をするとなるとヘルプの更新も必要になりそうですね。

ヘルプは今回触らないで良い気がします。

手動確認を考えていました。テスト結果のEvidenceが自己申告になりますが…。

何を確認したかが残れば、手動確認でよいです。
確認に使用したpythonスクリプトは必須ではないと思います。

このPRがマージされるまでに機能テストのPR(python必須)を出すかも知れませんが、連携させる必要はないと思ってます。

@AppVeyorBot
Copy link

Build sakura 1.0.4219 completed (commit 4498c98ac3 by @beru)

@AppVeyorBot
Copy link

Build sakura 1.0.4220 completed (commit 252c50badd by @beru)

@sonarcloud
Copy link

sonarcloud bot commented Dec 22, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell C 314 Code Smells

3.1% 3.1% Coverage
0.0% 0.0% Duplication

@AppVeyorBot
Copy link

Build sakura 1.0.4221 completed (commit d9ffc5fa5d by @beru)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ■機能追加
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants