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.

Sign up for GitHub

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

Jump to bottom

ツールバーの折り返しが機能しなくなる不具合を引き起こした変更をrevert #1314

Merged
merged 1 commit into from
May 31, 2020
Merged

ツールバーの折り返しが機能しなくなる不具合を引き起こした変更をrevert #1314

merged 1 commit into from
May 31, 2020

Conversation

beru
Copy link
Contributor

@beru beru commented May 25, 2020

PR の目的

ツールバーの折り返しが機能しないデグレを直すのが目的です。

カテゴリ

  • 不具合修正

テスト内容

#1313 の再現手順参照

関連 issue, PR

#1313, #456

@beru beru added the 🐛bug🦋 ■バグ修正(Something isn't working) label May 25, 2020
@AppVeyorBot
Copy link

✅ Build sakura 1.0.2845 completed (commit c7c60d1966 by @beru)

berryzplus
berryzplus reviewed May 30, 2020
View reviewed changes
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.

何が原因でツールバーの折り返しが機能しなくなったか調査しました？

@beru
Copy link
Contributor Author

beru commented May 30, 2020

何が原因でツールバーの折り返しが機能しなくなったか調査しました？

CMainToolBar::UpdateToolbar の実装を更新したんですがそこで、TBSTATE_ENABLED と TBSTATE_CHECKED 以外のステートを考慮せずに

WORD stateToSet = 0;

こう初期化してしまっていたのが原因でした。

WORD stateToSet = state & ~(TBSTATE_ENABLED | TBSTATE_CHECKED);

こうしないといけませんでした。

@beru beru added the 💩degradation🧻🚽 デグレ (前に動いていた機能が動かなくなった) label May 30, 2020
@m-tmatma
Copy link
Member

これは単に #456 を revert するものですよね?
#456 でやりたかったことは別 PR でやるんですか?

@beru
Copy link
Contributor Author

beru commented May 31, 2020

これは単に #456 を revert するものですよね?
#456 でやりたかったことは別 PR でやるんですか?

うーん、このPRで #1314 (comment) の対処を入れるのもいいかもしれないですね。

このPRを作成した時点ではとにかく急いでデグレを解消しようとしたんです。

@berryzplus
Copy link
Contributor

ぱっと見でそこがよく分からんかったのです。

原因にたどり着いているなら、原因に対処すべきだと思うんです。
どうすべきか？まで分かっているなら、何故 revert なんでしょうか。

@beru
Copy link
Contributor Author

beru commented May 31, 2020

高速化の変更を保つ事より不具合修正を優先した方が良いと思ったので原因を調査せずに不具合を引き起こした変更を revert したPRにしました。

@m-tmatma m-tmatma changed the title ツールバーの折り返しが機能しないデグレを修正 ツールバーの折り返しが機能しないデグレの修正を revert May 31, 2020
@m-tmatma
Copy link
Member

m-tmatma commented May 31, 2020 •
edited
Loading

タイトル変えました

@m-tmatma
Copy link
Member

根本原因を直しても、このまま revert するのでもいいと思います。

@beru
Copy link
Contributor Author

beru commented May 31, 2020

レビューありがとうございます。Mergeします。

#1314 (comment) の対応は別PRで行います。

@beru beru merged commit 6e75abe into sakura-editor:master May 31, 2020
@beru beru deleted the fix_toolbar_wrap branch May 31, 2020 10:26
This was referenced May 31, 2020
Merged
Closed
@berryzplus
Copy link
Contributor

これ、mergeすることに異論はありませんが、PRのタイトルおかしいような。

デグレの修正をrevert

実態は デグレを引き起こした修正をrevert なんだけど、
デグレを修正したのをrevertしたみたいに読める・・・

@m-tmatma m-tmatma changed the title ツールバーの折り返しが機能しないデグレの修正を revert ツールバーの折り返しが機能しないデグレを引き起こした修正をrevert Jun 3, 2020
@beru beru changed the title ツールバーの折り返しが機能しないデグレを引き起こした修正をrevert ツールバーの折り返しが機能しない不具合を引き起こした変更をrevert Jun 4, 2020
@beru beru changed the title ツールバーの折り返しが機能しない不具合を引き起こした変更をrevert ツールバーの折り返しが機能しなくなる不具合を引き起こした変更をrevert Jun 4, 2020
HoppingTappy pushed a commit to HoppingTappy/sakura that referenced this pull request Jun 16, 2020
@beru
Merge pull request sakura-editor#1314 from beru/fix_toolbar_wrap
bedc288 
ツールバーの折り返しが機能しないデグレの修正を revert
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@berryzplus berryzplus berryzplus left review comments

@m-tmatma m-tmatma m-tmatma approved these changes

Assignees
No one assigned
Labels
🐛bug🦋 ■バグ修正(Something isn't working) 💩degradation🧻🚽 デグレ (前に動いていた機能が動かなくなった)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@beru @AppVeyorBot @m-tmatma @berryzplus