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

SpWindowPresenter>>okToChange returns true incorrectly #15646

Closed
ericwinger opened this issue Nov 30, 2023 · 4 comments
Closed

SpWindowPresenter>>okToChange returns true incorrectly #15646

ericwinger opened this issue Nov 30, 2023 · 4 comments
Assignees

Comments

@ericwinger
Copy link

Bug description
I believe this code is incorrect

SpWindowPresenter>>okToChange

	self flag: #TODO. "Maybe wrong?"
	self presenter canDiscardEdits ifTrue: [ ^ true ].
	"Solicit cancel from view"
	self changed: #wantToChange.  
	^ self canDiscardEdits

To Reproduce
Implement a spec2 presenter where canDiscardEdits returns false and none of the window presenter's dependents returns false. The edits will be discarded anyway.

Expected behavior
If the presenter returns false from canDiscardEdits, return false from the method because, if I understand the methodology, either the presenter or one of the window presenter's dependents can veto the change.
I think ... self presenter canDiscardEdits ifFalse:[^false]. ... is correct.

Screenshots
None applicable

Version information:
Ubuntu 20.04

Pharo 11.0.0
Build information: Pharo-11.0.0+build.714.sha.0ead11d0b8573ff685db8a39fceeca2a8d528d3e (64 Bit)

Expected development cost
2 cents ... After all, it's just my opinion. :)

Additional context
None to add. It seems pretty straightforward.

Copy link

welcome bot commented Nov 30, 2023

Thanks for opening your first issue! Please check the CONTRIBUTING documents for some tips about which information should be provided. You can find information of how to do a Pull Request here: https://github.com/pharo-project/pharo/wiki/Contribute-a-fix-to-Pharo

GitHub
Pharo is a dynamic reflective pure object-oriented language supporting live programming inspired by Smalltalk. - pharo-project/pharo

ericwinger added a commit to GemTalk/JadeiteForPharo that referenced this issue Nov 30, 2023
Includes workaround pharo issue pharo-project/pharo#15646, #okToChange returning true incorrectly. 
See correct implementation in JadeiteWindowPrsenter>>okToChange. 

Rename okToChangeSelections to `canDiscardEdits` to conform with spec2 naming and simpler code.
@Ducasse
Copy link
Member

Ducasse commented Dec 12, 2023

Thanks for your report. I will link it to the issue in the Spec project.

@Ducasse
Copy link
Member

Ducasse commented Dec 12, 2023

pharo-spec/Spec#1492

@Ducasse Ducasse assigned estebanlm and unassigned estebanlm Dec 12, 2023
@Ducasse
Copy link
Member

Ducasse commented Dec 12, 2023

Hi eric

esteban replied on the spec issue. Please have a look. I will close this one to avoid duplication.

@Ducasse Ducasse closed this as completed Dec 12, 2023
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

No branches or pull requests

3 participants