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

grep で 除外ファイル、除外フォルダが効かない問題を修正するため、除外パターンを指定するコマンドラインを復活する #750

Merged
merged 1 commit into from
Jan 4, 2019

Conversation

m-tmatma
Copy link
Member

@m-tmatma m-tmatma commented Jan 4, 2019

#743: grep で 除外ファイル、除外フォルダが効かない問題を修正するため、除外パターンを指定するコマンドラインを復活する

#743 (comment)

Copy link
Contributor

@beru beru left a comment

Choose a reason for hiding this comment

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

不具合が修正された事を確認しました。

自分はコマンドラインのオプションが増えても良いんじゃないかと思うので Approve します。
たとえ既存のオプションを使って実現出来る事であっても、内容が明確なら入れてもメンテ出来るのではないかと…。ただ今後見直してオプションを整理したいという意見が出た場合にそれを否定する気は無いです。なんだかどっちつかずですが…。

ヘルプのコマンドラインオプションのページの更新が必要かと思いましたが、確認してみたら -GEXCLUDEFILE-GEXCLUDEFOLDER の説明は残ったままなので大丈夫ですね。 これは残ったままじゃなくてこのPRで復活したんですね。

@beru
Copy link
Contributor

beru commented Jan 4, 2019

今回の変更は GUI でのGrep操作も場合によってはコマンドライン経由になるという事を忘却してて、それで必要になった感じですね。作りを変えてコマンドライン経由じゃなくする、というのも取りうる対処方法の一つかもしれませんが改造が大変そうかなと思います。

@m-tmatma m-tmatma added the 🐛bug🦋 ■バグ修正(Something isn't working) label Jan 4, 2019
@m-tmatma m-tmatma added this to the next release milestone Jan 4, 2019
@m-tmatma
Copy link
Member Author

m-tmatma commented Jan 4, 2019

マージしちゃいます

@m-tmatma m-tmatma merged commit 3a47bf7 into sakura-editor:master Jan 4, 2019
@ds14050
Copy link
Contributor

ds14050 commented Jan 4, 2019

たとえ既存のオプションを使って実現出来る事であっても、内容が明確なら入れてもメンテ出来るのではないかと…。ただ今後見直してオプションを整理したいという意見が出た場合にそれを否定する気は無いです。

入れたならば、コマンドラインオプションの見直しには反対します。

別の場所でも触れましたが、コマンドラインオプションはレジストリに記録することもありますから GUI とはライフサイクルが違います。「これは内部仕様だから変更されることがあります、こちらのオプションを使ってこう書いてください」と書いておけば言い訳だけは立ちますけれど、無理でしょう。自分だって個別直接的なオプションを使います。

@beru
Copy link
Contributor

beru commented Jan 4, 2019

なるほど、確かに互換性は大事かもしれないですね。
となると方向修正出来るとしたらリリースまだしてない今のうちでしょうか…。

@berryzplus
Copy link
Contributor

これは時間を置いてからマージして欲しかったです。
入れてしまったもんはしょうがないので「revertしろ」とは言いません。
これがあったからといって問題が悪化するわけではないと思っています。

@m-tmatma
Copy link
Member Author

m-tmatma commented Jan 4, 2019

別にやり直すのは手間ではないので、やり直したほうが良ければ、良いですよ。

@m-tmatma
Copy link
Member Author

m-tmatma commented Jan 4, 2019

#753 を作成しました

@beru
Copy link
Contributor

beru commented Jan 5, 2019

@berryzplus さん

これは時間を置いてからマージして欲しかったです。
入れてしまったもんはしょうがないので「revertしろ」とは言いません。
これがあったからといって問題が悪化するわけではないと思っています。

別の対処方法の案はお持ちですか?
なるべく手間がかからない方法が良いですね。
出来れば実装してもらえるとなお…。

@m-tmatma m-tmatma added the Won't Fix 対応しない【ChangeLog除外】 label Jan 5, 2019
HoppingTappy pushed a commit to HoppingTappy/sakura that referenced this pull request Jun 11, 2019
…rep-bug

grep で 除外ファイル、除外フォルダが効かない問題を修正するため、除外パターンを指定するコマンドラインを復活する
@m-tmatma m-tmatma deleted the feature/issue743-grep-bug branch July 5, 2019 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛bug🦋 ■バグ修正(Something isn't working) Won't Fix 対応しない【ChangeLog除外】
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants