Skip to content

Bug fix for editor content loss#240

Merged
Chris7 merged 1 commit intoMudlet:release_30from
Chris7:searchWidgetBug
Jan 18, 2015
Merged

Bug fix for editor content loss#240
Chris7 merged 1 commit intoMudlet:release_30from
Chris7:searchWidgetBug

Conversation

@Chris7
Copy link
Copy Markdown
Member

@Chris7 Chris7 commented Jan 13, 2015

This fixes the bug where moving through the searches will cause content to be overwritten.

@vadi2
Copy link
Copy Markdown
Member

vadi2 commented Jan 13, 2015

Does seem to work, but I'll be running this for a few days to confirm.

On Wed, Jan 14, 2015 at 4:16 AM, Chris Mitchell notifications@github.com
wrote:

This fixes the bug where moving through the searches will cause content to

be overwritten.

You can merge this Pull Request by running

git pull https://github.com/Chris7/Mudlet searchWidgetBug

Or view, comment on, or merge it at:

#240
Commit Summary

  • Bug fix for editor content loss

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#240.

@SlySven
Copy link
Copy Markdown
Member

SlySven commented Jan 16, 2015

Chris7 wrote:

I think an isDirty flag is a hack and would rather the methods be simplified and more concise.

I'll be honest and say I agree with the second half of that sentence, but I remain unconvinced that we can accept signals from different points (toolbar button click - item selected - tree selection changed by keyboard etc.) as we will get in this "event"-driven environment without tracking whether we have already responded as necessary in response to a different signal that needs the same process to be carried out.
I note that we track various things in dlgTriggerEditor:

  • mpCurrent{Action|Alias|Key|Script|Timer|Trigger|Var}Item - only the Var get used as a RValue
  • mCurrent{Action|Alias|Key|Script|Timer|Trigger|Var} - prefix should be "mp" not "m" !
  • mp{Action|Alias|Key|Script|Timer|Trigger|Var}BaseItem

I don't think all of these QTreeWidgetItem pointers are being used as intended - the first type is largely unused in fact.

Chris7 added a commit that referenced this pull request Jan 18, 2015
Bug fix for editor content loss
@Chris7 Chris7 merged commit 901176c into Mudlet:release_30 Jan 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants