The Trunk: Morphic-cmm.1052.mcz

Chris Muller uploaded a new version of Morphic to project The Trunk:
http://source.squeak.org/trunk/Morphic-cmm.1052.mcz

==================== Summary ====================

Name: Morphic-cmm.1052
Author: cmm
Time: 21 November 2015, 11:23:10.034 am
UUID: 984e80fd-459b-467b-b350-bff00bb77b60
Ancestors: Morphic-mt.1051

- Restore the use-case "Replace Expression With A New Expression" (e.g., fix autoEnclose).
        "Connect Existing Expression To A New Expression" is already present via the Command key.
        We need to support both of those use cases.

=============== Diff against Morphic-mt.1051 ===============

Item was changed:
  ----- Method: TextEditor>>autoEncloseFor: (in category 'typing support') -----
  autoEncloseFor: typedChar
  "Answer whether typeChar was handled by auto-enclosure.  Caller should call normalCharacter if not."
  | openers closers |
  openers := '([{'.
  closers := ')]}'.
  (closers includes: typedChar) ifTrue:
  [ | pos |
  self blinkPrevParen: typedChar.
  ((pos := self indexOfNextNonwhitespaceCharacter) notNil and: [ (paragraph string at: pos) = typedChar ])
  ifTrue:
  [ self
  moveCursor: [ : position | position + pos - pointBlock stringIndex + 1 ]
  forward: true
  select: false.
  ^ true ]
  ifFalse: [ ^ false ] ].
+ (self class autoEnclose and: [ openers includes: typedChar ]) ifTrue:
+ [ self
+ addString: (closers at: (openers indexOf: typedChar)) asString ;
+ insertTypeAhead ;
- (openers includes: typedChar) ifTrue: [
- self
- openTypeIn;
- addString: typedChar asString;
- addString: (closers at: (openers indexOf: typedChar)) asString ;
- insertAndCloseTypeIn ;
 
  moveCursor: [ : position | position - 1 ]
  forward: false
  select: false.
+ ^ false ].
- ^ true ].
  ^ false!

Item was changed:
  ----- Method: TextEditor>>dispatchOnKeyboardEvent: (in category 'typing support') -----
  dispatchOnKeyboardEvent: aKeyboardEvent
  "Carry out the action associated with this character, if any. Type-ahead is passed so some routines can flush or use it."
 
  | honorCommandKeys typedChar |
  typedChar := aKeyboardEvent keyCharacter.
 
  "Create a new command for separating characters. Do not create separate commands for consecutive separators."
  ((Character separators includes: typedChar)
  and: [(Character separators includes: previousKeyCharacter) not])
  ifTrue: [self closeTypeIn].
  previousKeyCharacter := typedChar.
 
  "Handle one-line input fields."
  (typedChar == Character cr and: [morph acceptOnCR])
  ifTrue: [^ true].
 
  "Clear highlight for last opened parenthesis."
  self clearParens.
 
  "Handle line breaks and auto indent."
  typedChar == Character cr ifTrue: [
  aKeyboardEvent controlKeyPressed
  ifTrue: [^ self normalCharacter: aKeyboardEvent].
  aKeyboardEvent shiftPressed
  ifTrue: [^ self lf: aKeyboardEvent].
  aKeyboardEvent commandKeyPressed
  ifTrue: [^ self crlf: aKeyboardEvent].
  ^ self crWithIndent: aKeyboardEvent].
 
  "Handle indent/outdent with selected text block."
  typedChar == Character tab ifTrue: [
  aKeyboardEvent shiftPressed
  ifTrue: [self outdent: aKeyboardEvent. ^ true]
  ifFalse: [self hasMultipleLinesSelected
  ifTrue: [self indent: aKeyboardEvent. ^ true]]].
 
  honorCommandKeys := Preferences cmdKeysInText.
 
  (honorCommandKeys and: [typedChar == Character enter])
  ifTrue: [^ self dispatchOnEnterWith: aKeyboardEvent].
 
  "Special keys overwrite crtl+key combinations - at least on Windows. To resolve this
  conflict, assume that keys other than cursor keys aren't used together with Crtl."
  ((self class specialShiftCmdKeys includes: aKeyboardEvent keyValue)
  and: [aKeyboardEvent keyValue < 27])
  ifTrue: [^ aKeyboardEvent controlKeyPressed
  ifTrue: [self
  perform: (self class shiftCmdActions at: aKeyboardEvent keyValue + 1)
  with: aKeyboardEvent]
  ifFalse: [self
  perform: (self class cmdActions at: aKeyboardEvent keyValue + 1)
  with: aKeyboardEvent]].
 
  "backspace, and escape keys (ascii 8 and 27) are command keys"
  ((honorCommandKeys and: [aKeyboardEvent commandKeyPressed])
  or: [self class specialShiftCmdKeys includes: aKeyboardEvent keyValue])
  ifTrue: [ ^ aKeyboardEvent shiftPressed
  ifTrue: [self
  perform: (self class shiftCmdActions at: aKeyboardEvent keyValue + 1)
  with: aKeyboardEvent]
  ifFalse: [self
  perform: (self class cmdActions at: aKeyboardEvent keyValue + 1)
  with: aKeyboardEvent]].
 
  "the control key can be used to invoke shift-cmd shortcuts"
  (honorCommandKeys and: [ aKeyboardEvent controlKeyPressed ])
  ifTrue: [^ self
  perform: (self class shiftCmdActions at: aKeyboardEvent keyValue + 1)
  with: aKeyboardEvent].
 
- "Automatically enclose paired characters such as brackets."
  self class autoEnclose
+ ifTrue:
+ [ (self autoEncloseFor: typedChar) ifFalse: [ self normalCharacter: aKeyboardEvent ] ]
+ ifFalse: [ self normalCharacter: aKeyboardEvent ].
+
- ifTrue: [((self hasSelection and: [self enclose: aKeyboardEvent])
- or: [self autoEncloseFor: typedChar])
- ifTrue: [^ true]].
-
- self normalCharacter: aKeyboardEvent.
  ^ false!

-1

1. This breaks the undo for #autoEnclose, which I had fixed on purpose.
2. This removes a very convenient feature to put brackets around the current selection.

Please revert that commit or give a rationale. ;-) Or both.

What is that use case "replace expression with new expression"? Just from reading the words, it just worked fine.

Best,
Marcel

Hi Marcel, I hope you can find another solution to the Undo.  The
change that was made to autoEnclose introduced several bugs, whilst
nullifying one use case and overloading another.

User-expectations about selection-replacement were violated -- we just
talked about this for the Tab key.  When the user types a regular
character into the Editor, they expect that character to be typed.  If
the user wishes to modify that expected behavior, they may use one of
the modifier keys.

With your change, I could not replace any selection with a 9, (, [, {,
' or ".  , because "Replace Expression With A New Expression" was
eliminated, while at the same time "Connect Existing Expression To A
New Expression" was overloaded (introduced a second ways to accomplish
the same thing).


On Sat, Nov 21, 2015 at 11:15 AM, marcel.taeumel <[hidden email]> wrote:

This sounds like the sort of change that might benefit from a couple of
days in the inbox.

Dave

On Sat, Nov 21, 2015 at 11:49:54AM -0600, Chris Muller wrote:

Hi Chris,

I fixed undo again: http://forum.world.st/The-Trunk-Morphic-mt-1053-mcz-td4862470.html

Now we can continue this discussion.

1) I understand that that magical enclosing selection violates that "replace selection with new expression" principle. We might consider adding a preference for that because I just cannot recall those hidden enclose keyboard shotcuts. Having brackets enclosing selections this feels much more convenient. Keeps the flow, you know. ;-) For now, it's gone again. What a pity.

2) Why did you break the undo for autoEnclose in the first place? Was it a slip? Maybe I misunderstood "connect existing expression to a new expression" but the way TextEditor >> #autoEncloseFor: is now (with undo working again) it does not seem to harm anything. You must not modify the text cursor while just typing in (or calling #addString:). See #closeTypeIn for explanation. Btw: The (old/mini) undo in 5.0 was also broken for autoEnclose. That additional closing char did not disappear. --- However, it works now.

Does this cover it? Introducing additional ways to do the same is very common in user interface design. It's not just about novices and experts. For example, there is CMD+V and the context menu with "Paste it". To ways to achieve the same thing. :)

Best,
Marcel