Opened 7 years ago
Last modified 7 years ago
#165 testing defect
Not all spaces are created equal
Reported by: | Owned by: | ||
---|---|---|---|
Priority: | major | Milestone: | |
Component: | default | Keywords: | |
Cc: | Also affects CVS HEAD (eXept version): | no |
Description
This issue is a tricky one. When a space is created between two strings (e.g. two words) and then enter is pressed then this spaces represents end of the line and end of the string. If you create additional spaces via spacebar or just arrow keys you can not influence the length of such string. Even pressing END key returns to this invisible end of a line. All other keys like delete or backspace still see this as end of the line.
Not in a case that you create the space via spacebar such space is just ignored and deleted - e.g. via backspace and ignored by end of line keys like key END.
More to see on the attached video
Attachments (5)
Change History (11)
by , 7 years ago
Attachment: | spaces_end_key_behavior_on_unpatched_stx.mp4 added |
---|
comment:1 by , 7 years ago
In other words these two tests should be the same but are not! (note: these tests are already using the patch from #162)
This is the successful test:
test_03a "This tests if there are spaces left at the end after moving the line up" textView contents:'This text is tocontinue'. textView setCursorLine:1. textView setCursorCol:16. textViewInteractor type: Character space. textViewInteractor type: Character space. textViewInteractor type: Character space. textView setCursorLine:2. textView setCursorCol:1. textViewInteractor type: #BackSpace. textView lineEndCRLF: false. self assert:textView contents asString = 'This text is to continue'.
On the other hand if you do it this way:
test_03b "This tests if there are spaces left at the end after moving the line up" textView contents:'This text is to', String crlf, 'continue'. textView setCursorLine:1. textView setCursorCol:16. textViewInteractor type: Character space. textViewInteractor type: Character space. textViewInteractor type: Character space. textView setCursorLine:2. textView setCursorCol:1. textViewInteractor type: #BackSpace. textView lineEndCRLF: false. self assert:textView contents asString = 'This text is to continue'.
The test fails!!
comment:2 by , 7 years ago
Two more tests:
A passing one - the space is preserved when the enter key is pressed and spaces are moved together with the text:
test_03d "Test to check if the spaces at the beginning of the text are preserved when backspace is pressed" textViewInteractor type: Character space. textViewInteractor type: Character space. textViewInteractor type: Character space. textViewInteractor type: 'test'. textView setCursorCol: 1. textViewInteractor type: #Return. textView setCursorLine: 2. textView setCursorCol: 1. textViewInteractor type: #BackSpace. textViewInteractor type: #BackSpace. self assert:textView contents asString = (' test', Character cr). "Created: / 17-08-2017 / 09:33:14 / svestkap"
A failing test - the spaces are left alone and the text is moved by pressing enter to the next line. Then a backspace is pressed and spaces magically disappear:
test_03c "Test to check if the spaces at the beginning of the text are preserved when backspace is pressed" textViewInteractor type: Character space. textViewInteractor type: Character space. textViewInteractor type: Character space. textViewInteractor type: 'test'. textView setCursorCol: 4. textViewInteractor type: #Return. textView setCursorLine: 2. textView setCursorCol: 1. textViewInteractor type: #BackSpace. textViewInteractor type: #BackSpace. self assert:textView contents asString = (' test', Character cr). "Created: / 17-08-2017 / 09:23:10 / svestkap"
comment:3 by , 7 years ago
Status: | new → testing |
---|
I have create a remedy for the white space issue.
The patch contains 3 patches:
- libbasic to remedy return value
- libtool to add new option for trimBlankLines (default value is same as before)
- libwidg contain tests for the new & old functionality to make sure the editor works properly with the new setting
by , 7 years ago
Attachment: | libbasic_fix_1_of_1_rev_715bb22ed5cd_Issue__165__Not_all_spaces_are_created_equal.patch added |
---|
fixing the return value
by , 7 years ago
Attachment: | libtool_fix_1_of_1_rev_b5bcb51d114a_Issue__165__Not_all_spaces_are_created_equal.patch added |
---|
Adding new trimBlankLines option
by , 7 years ago
Attachment: | libwidg_fix_1_of_1_rev_4dc6edccd6f7_Issue__165__Not_all_spaces_are_created_equal.patch added |
---|
Test cases for the new functionality
by , 7 years ago
comment:4 by , 7 years ago
The patch in stx:libbasic is not needed (and actually it's "wrong"). UserPreferences >> #trimBlankLines:
is a "setter" so it shall not return value.
If you want to make sure that UserPreferences current trimBlankLines
returns true
in the test no matter what, why not doing:
MessageTracer mock: #trimBlankLines in: UserPreferences do:[ true ].
However, you need not mock it at all, all you need in this case is to set the option in text view its
elf:
textView trimBlankLines: true
See EditTextView >> trimBlankLines
and EditTextView >> trimBlankLines:
. I changed the tests accordingly, see my patch 7e214afed3b8.
comment:5 by , 7 years ago
What you show in the attached video can be seen as "weird" but this is - IMO - a correct behavior w.r.t. "sheet of paper" metaphore used in Smalltalk/X editor.
If you're going to fix this (and other things), we would need a new setting to switch between "sheet of paper" behavior and "all the others" behavior. Then the editor should behave depending on the setting. This may make things more complex but that;s the only way for non-technical reasons.
Now, the ticket is in "testing". I'm not sure what to do. If I approve and integrate modified patch with tests and also the patch to libtool (may need little modifications too), would you consider this issue solved? If not, what are the next steps?
comment:6 by , 7 years ago
Maybe the best approach would be to create a special setting for "mode: sheet paper"
and then separate setting for "mode: text editor"
. This setting would then belong to the text editor mode
. Then this would be sub-setting for the text editor mode
(this ticket would be then a sub-ticket to this approach.)
What do you think about this solution?
See the funny thing about spaces at StX