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

Fix confusion between clear and clearAll in TextEdit hierarchy #1220

Merged
merged 1 commit into from
Dec 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Fix confusion between clear and clearAll in TextEdit hierarchy
- clear resets the control to empty. It is not a command.
- clearAll is an undoable command to delete all text
  • Loading branch information
blairmcg committed Dec 17, 2023
commit 485612df4fbec5727d938068ed06c987eb2ee94d
2 changes: 1 addition & 1 deletion Core/Object Arts/Dolphin/IDE/Base/Tools.Debugger.cls
Original file line number Diff line number Diff line change
Expand Up @@ -1038,7 +1038,7 @@ onTempSelected
ifNil:
[self displayLocal: nil.
inspectorPresenter view
clearAll;
clear;
disable]!

onTerminate
Expand Down
40 changes: 18 additions & 22 deletions Core/Object Arts/Dolphin/IDE/Base/UI.TranscriptShell.cls
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,8 @@ addToCommandRoute: route
to go next.
Implementation Note: We want to include the development system if it is present."

route
appendPresenter: self;
appendTarget: Smalltalk developmentSystem!
super addToCommandRoute: route.
route appendTarget: Smalltalk developmentSystem!

alertUser
"Private - Attempt to catch the users attention to inform them that output has been
Expand All @@ -61,9 +60,10 @@ clear
clearAll
"Remove all contents in the receiver's view"

<commandQuery: #hasOutput>
| newBuffer |
self flush.
self hasOutputWindow ifTrue: [outputWindow clear].
self hasOutputWindow ifTrue: [outputWindow clearAll].
newBuffer := String smalltalkWriteStream.
bufferProtect critical: [buffer := newBuffer]!

Expand Down Expand Up @@ -147,13 +147,20 @@ flush
forgetSize
"Forget the default size for new instances of this tool."

<commandQuery: #hasRememberedSize>
self class defaultExtent: nil!

hasOutput
^buffer position ~~ 0 or: [outputWindow textLength > 0]!

hasOutputWindow
"Private - Answer whether the receiver has an open output window available."

^outputWindow isOpen == true!

hasRememberedSize
^DefaultExtent notNil!

initialize
"Private - Initialize the receiver."

Expand Down Expand Up @@ -237,18 +244,6 @@ print: anObject

bufferProtect critical: [anObject printOn: buffer]!

queryCommand: aCommandQuery
"Private - Enter details about a potential command for the receiver into the
<CommandQuery>."

| command |
command := aCommandQuery commandSymbol.
#forgetSize == command
ifTrue:
[aCommandQuery isEnabled: self class defaultExtent notNil.
^true].
^super queryCommand: aCommandQuery!

rememberThisSize
"Record the size of the receiver as the default extent for its tool class."

Expand Down Expand Up @@ -321,11 +316,11 @@ visit: anObject do: aNiladicValuable
aNiladicValuable value! !
!UI.TranscriptShell categoriesForMethods!
<<!public! !
addToCommandRoute:!commands!public! !
addToCommandRoute:!commands-routing!public! !
alertUser!operations!private! !
basicPrint:!printing!private! !
clear!initializing!public! !
clearAll!initializing!public! !
clearAll!commands-actions!public! !
close!operations!public! !
contents!accessing!public! !
cr!adding!public! !
Expand All @@ -336,10 +331,12 @@ crtab:!adding!public! !
defaultHelpId!public! !
display:!printing!public! !
encodedSizeOf:!public! !
fileFileIn!commands!public! !
fileFileIn!commands-actions!public! !
flush!buffer!public! !
forgetSize!commands!public! !
forgetSize!commands-actions!public! !
hasOutput!commands-queries!private! !
hasOutputWindow!accessing!private! !
hasRememberedSize!commands-queries!private! !
initialize!initializing!private! !
isEmpty!public!testing! !
locale!accessing!public! !
Expand All @@ -353,8 +350,7 @@ onViewOpened!event handling!private! !
position!accessing!public! !
position:!accessing!public! !
print:!printing!public! !
queryCommand:!commands!private! !
rememberThisSize!commands!public! !
rememberThisSize!commands-actions!public! !
reset!positioning!public! !
setToEnd!operations!public! !
show!operations!public! !
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ package methodNames
add: #SystemMetrics -> #hasMenuImages;
add: #SystemMetrics -> #hasTreeViewHotTracking;
add: #SystemMetrics -> #supportsAlphaBlending;
add: #TextEdit -> #clear;
add: #TextEdit -> #findNextWrapped:down:wholeWord:matchCase:;
add: #TextEdit -> #isLowercase;
add: #TextEdit -> #isLowercase:;
Expand Down Expand Up @@ -946,13 +945,6 @@ supportsAlphaBlending!capability enquiries!public! !

!TextEdit methodsFor!

clear
"Clears the contents of the receiver"

<commandQuery: #canCut>
Notification deprecated. "Use #clearAll"
self clearAll!

findNextWrapped: aString down: downBoolean wholeWord: wordBoolean matchCase: caseBoolean
#deprecated.
self findNextWrapped: (FindDetails new
Expand Down Expand Up @@ -1006,7 +998,6 @@ updatePerChar: aBoolean
Notification deprecated.
self updateMode: (aBoolean ifTrue: [#perChar] ifFalse: [#focusLost])! !
!TextEdit categoriesForMethods!
clear!commands-actions!public! !
findNextWrapped:down:wholeWord:matchCase:!public!searching & replacing! !
isLowercase!public!testing! !
isLowercase:!public!testing! !
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,11 +274,15 @@ setTabStops: anInteger
wParam: 1
lpParam: width yourAddress!

showVerticalScrollBar: aBoolean
(self
showVerticalScrollBar: aBoolean
"Add/remove vertical scrollbar."

"As noted in the [edit control documentation](https://learn.microsoft.com/en-us/windows/win32/controls/edit-control-styles), the styles cannot generally be set are creation, so we need to recreate the control to add/remove ES_AUTOVSCROLL."

(self
baseStyleMask: ##(WS_VSCROLL | ES_AUTOVSCROLL)
set: aBoolean
recreateIfChanged: false) ifTrue: [self frameChanged]!
recreateIfChanged: true) ifTrue: [self frameChanged]!

tabFocus
"Private - Sets focus to the receiver. Answers the View which previously had focus,
Expand Down
21 changes: 14 additions & 7 deletions Core/Object Arts/Dolphin/MVP/Presenters/Text/UI.TextEdit.cls
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@ apply
^modified!

basicClearAll
self value: ''!
self
basicSelectAll;
basicClearSelection!

basicClearSelection
self sendMessage: WM_CLEAR!
Expand Down Expand Up @@ -120,7 +122,7 @@ basicSelectionStart: start end: end
self sendMessage: EM_SETSEL wParam: start lParam: end!

basicUndo
self sendMessage: EM_UNDO!
^(self sendMessage: EM_UNDO) asBoolean!

calcRectangleFromClientRectangle: aRectangle
"Private - Given a client rectangle represented by the <Rectangle> argument, answer the
Expand Down Expand Up @@ -213,8 +215,13 @@ charNearestPosition: aPoint
wParam: 0
lParam: pt asUIntPtr) lowWord + 1!

clear
"Reset the receiver to an empty state; no text, empty undo buffer, not modified. See also the #clearAll command (which clears the text, but as an undoable action)"

self value: ''!

clearAll
"Clears the contents of the receiver"
"Delete all the text in the receiver."

<commandQuery: #canCut>
self basicClearAll!
Expand Down Expand Up @@ -582,8 +589,7 @@ isModified: aBoolean
may be lost."

aBoolean ifFalse: [self updateModel].
teFlags := teFlags mask: ModifiedMask set: aBoolean.
self assert: [self isModified == aBoolean]!
teFlags := teFlags mask: ModifiedMask set: aBoolean!

isPassword
"Answer whether the receiver is a password entry field (i.e. it has the
Expand Down Expand Up @@ -1164,10 +1170,10 @@ textRange
!

undo
"Undo the last undoable edit action."
"Undo the last undoable edit action. Answer an undo took place (although for a single-line Edit control, this will always claim to be true)"

<commandQuery: #canUndo>
self basicUndo!
^self basicUndo!

updateMode
"Answer the update mode of the receiver:
Expand Down Expand Up @@ -1322,6 +1328,7 @@ canUndo!public!testing!undo & redo! !
caretPosition!caret!public! !
caretPosition:!accessing!caret!public! !
charNearestPosition:!accessing!public! !
clear!commands-actions!public! !
clearAll!commands-actions!public! !
clearSelection!clipboard operations!commands-actions!public! !
command:id:!commands-routing!private! !
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,53 @@ UI.Tests.AbstractTextEditTest isNonInstantiable: true!
UI.Tests.AbstractTextEditTest comment: ''!
!UI.Tests.AbstractTextEditTest methodsFor!

initializePresenter
super initializePresenter.
presenter text: self text!

margins
^self subclassResponsibility!

testApply
self deny: presenter isModified.
self deny: presenter isTextModified.
presenter selectionRange: (1 to: 1).
presenter cutSelection.
self assert: presenter isTextModified.
self assert: presenter isModified.
presenter apply.
"The isModified flag indicates that the value has been updated since originally set - it doesn't necessarily mean the model has not been updated. This has to be manually reset when the value is saved (in whatever manner is used to persist it externally) #apply doesn't clear this flag, in fact it sets it if there were pending changes in the view."
self assert: presenter isModified.
"The isTextModified flag indicates that he value in the view is updated with respect to the model"
self deny: presenter isTextModified!

testClearing
"There are two 'clearing' operations:
- #clearAll - a command to delete all the text in the control, leaving it in a modified (and undoable) state
- #clear - reset the control to empty, leaving unmodified and with an empty undo stack."

self assert: presenter plainText equals: self text.
self deny: presenter isTextModified.
self deny: presenter canUndo.
presenter clearAll.
self assert: presenter plainText isEmpty.
self assert: presenter isTextModified.
self assert: presenter canUndo.
self undoOnlyModification.
presenter clear.
self assert: presenter plainText isEmpty.
self deny: presenter isTextModified.
self deny: presenter canUndo!

testPositionOfChar
| text canvas lineHeight metrics i |
text := self text.
"Use a non-proportional font to avoid variability introduced by kerning, especially of punctuation."
| canvas lineHeight metrics i |
presenter view font: self nonProportionalFont.
canvas := presenter view canvas.
canvas font: presenter view actualFont.
metrics := canvas textMetrics.
lineHeight := metrics tmHeight.
presenter text: text.
i := 0.
text do:
self text do:
[:ch |
| line lineText startOfLine point extent |
i := i + 1.
Expand All @@ -46,6 +78,11 @@ testPositionOfChar
text
^self subclassResponsibility!

undoOnlyModification
self assert: presenter undo.
self assert: presenter plainText equals: self text
"Standard Edit control has only single-level undo, so it can't tell if the undo has cleared all modifications, or just reverted the last one"!

verifyUpgradedView: anInteger identifier: aResourceIdentifier
| view |
super verifyUpgradedView: anInteger identifier: aResourceIdentifier.
Expand All @@ -59,9 +96,13 @@ verifyUpgradedView: anInteger identifier: aResourceIdentifier
self assert: view interactor identicalTo: view.
self assertIsNil: view forecolor! !
!UI.Tests.AbstractTextEditTest categoriesForMethods!
initializePresenter!public!Running! !
margins!constants!private! !
testApply!public! !
testClearing!public!unit tests! !
testPositionOfChar!public!unit tests! !
text!constants!private! !
undoOnlyModification!helpers!private! !
verifyUpgradedView:identifier:!helpers!private! !
!

Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@ classToTest
margins
^1 @ 1!

testClearing
"Multiline-edit control will delete text restored through undo if it exceeds the display height and/or width and horizontal/vertical scrolling styles are not set."

presenter
canVScroll: true;
canHScroll: true.
super testClearing!

verifyUpgradedView: anInteger identifier: aResourceIdentifier
| view |
super verifyUpgradedView: anInteger identifier: aResourceIdentifier.
Expand All @@ -26,6 +34,7 @@ verifyUpgradedView: anInteger identifier: aResourceIdentifier
!UI.Tests.MultilineTextEditTest categoriesForMethods!
classToTest!constants!private! !
margins!constants!private! !
testClearing!public!unit tests! !
verifyUpgradedView:identifier:!helpers!private! !
!

Expand Down
7 changes: 7 additions & 0 deletions Core/Object Arts/Dolphin/MVP/Tests/UI.Tests.TextEditTest.cls
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ testCalculateExtent
"The width calculated by the GDI GetTextExtentPoint32 function is slightly different than the DrawTextEx calculation used by the control (and now#calculateExtent:) so we don't expect a precise result"
self assert: (actual x - expected x) abs < 3!

testClearing
"Single-line edit control will truncate text that exceeds the display area unless ES_AUTOHSCROLL is set."

presenter canHScroll: true.
super testClearing!

text
^'The quick brown fox jumped over the lazy dog'!

Expand All @@ -67,6 +73,7 @@ verifyUpgradedView: anInteger identifier: aResourceIdentifier
classToTest!constants!private! !
margins!constants!private! !
testCalculateExtent!public!unit tests! !
testClearing!public!unit tests! !
text!constants!private! !
verifyUpgradedView:identifier:!helpers!private! !
!
Expand Down
Loading