Discussion:
The Trunk: Multilingual-dtl.233.mcz
(too old to reply)
c***@source.squeak.org
0000-01-23 21:54:16 UTC
Permalink
David T. Lewis uploaded a new version of Multilingual to project The Trunk:
http://source.squeak.org/trunk/Multilingual-dtl.233.mcz

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

Name: Multilingual-dtl.233
Author: dtl
Time: 21 January 2018, 11:03:22.374228 am
UUID: 271b4b88-c037-4669-b2d2-15375755dcb4
Ancestors: Multilingual-pre.232

MutliByteFileStream>>upToPosition: fix provided by Bob Arning.

See squeak-dev discussion thread "MultiByteFileStream upToAll: strange bug" for background and diagnosis. The problem was introduced in Multilingual-tonyg.218 and merged to trunk in Multilingual-pre.230

Condensed email from Bob:

To: squeak-***@lists.squeakfoundation.org
From: Bob Arning
Date: Sun, 21 Jan 2018 07:01:37 -0500
Subject: Re: [squeak-dev] MultiByteFileStream upToAll: strange bug

The culprit is MultiByteFileStream>>upToPosition: which was referencing the instVar <position> directly. Changing that to "self position" allows it to stop at the right place.

=============== Diff against Multilingual-pre.232 ===============

Item was changed:
----- Method: MultiByteFileStream>>upToPosition: (in category 'accessing') -----
upToPosition: anInteger
"Answer a subcollection containing items starting from the current position and ending including the given position. Usefully different to #next: in that positions measure *bytes* from the file, where #next: wants to measure *characters*."
^self collectionSpecies new: 1000 streamContents: [ :stream |
| ch |
+ [ (ch := self next) == nil or: [ self position > anInteger ] ]
- [ (ch := self next) == nil or: [ position > anInteger ] ]
whileFalse: [ stream nextP
Tobias Pape
2018-01-21 18:48:31 UTC
Permalink
Cool!

Since Multilingual-tonyg.218 was part of a fix for Mantis#4665 (bugs.squeak.org/bug_view_advanced_page.php?bug_id=4665),
can we make sure that we don't have a regression because of that?
:)

Best regards
-Tobias
Post by c***@source.squeak.org
http://source.squeak.org/trunk/Multilingual-dtl.233.mcz
==================== Summary ====================
Name: Multilingual-dtl.233
Author: dtl
Time: 21 January 2018, 11:03:22.374228 am
UUID: 271b4b88-c037-4669-b2d2-15375755dcb4
Ancestors: Multilingual-pre.232
MutliByteFileStream>>upToPosition: fix provided by Bob Arning.
See squeak-dev discussion thread "MultiByteFileStream upToAll: strange bug" for background and diagnosis. The problem was introduced in Multilingual-tonyg.218 and merged to trunk in Multilingual-pre.230
From: Bob Arning
Date: Sun, 21 Jan 2018 07:01:37 -0500
Subject: Re: [squeak-dev] MultiByteFileStream upToAll: strange bug
The culprit is MultiByteFileStream>>upToPosition: which was referencing the instVar <position> directly. Changing that to "self position" allows it to stop at the right place.
=============== Diff against Multilingual-pre.232 ===============
----- Method: MultiByteFileStream>>upToPosition: (in category 'accessing') -----
upToPosition: anInteger
"Answer a subcollection containing items starting from the current position and ending including the given position. Usefully different to #next: in that positions measure *bytes* from the file, where #next: wants to measure *characters*."
^self collectionSpecies new: 1000 streamContents: [ :stream |
| ch |
+ [ (ch := self next) == nil or: [ self position > anInteger ] ]
- [ (ch := self next) == nil or: [ position > anInteger ] ]
whileFalse: [ stream
David T. Lewis
2018-01-21 19:10:37 UTC
Permalink
Post by Tobias Pape
Cool!
Since Multilingual-tonyg.218 was part of a fix for Mantis#4665 (bugs.squeak.org/bug_view_advanced_page.php?bug_id=4665),
can we make sure that we don't have a regression because of that?
:)
There were two unit tests provided in the bug report, both of which are in trunk:

(MultiByteFileStreamTest selector: #testUpToAllAscii) run
==> 1 run, 1 passes, 0 expected failures, 0 failures, 0 errors, 0 unexpected passes


(MultiByteFileStreamTest selector: #testUpToAllUtf) run
==> 1 run, 1 passes, 0 expected failures, 0 failures, 0 errors, 0 unexpected passes


So if you believe the unit tests, then there is no regression :-)

I am not the best person to be looking into multilingual issues, inasmuch
as my formal education ended with seven bit ASCII. It would be good if
someone else can double check this too.

Dave
Post by Tobias Pape
Best regards
-Tobias
Post by c***@source.squeak.org
http://source.squeak.org/trunk/Multilingual-dtl.233.mcz
==================== Summary ====================
Name: Multilingual-dtl.233
Author: dtl
Time: 21 January 2018, 11:03:22.374228 am
UUID: 271b4b88-c037-4669-b2d2-15375755dcb4
Ancestors: Multilingual-pre.232
MutliByteFileStream>>upToPosition: fix provided by Bob Arning.
See squeak-dev discussion thread "MultiByteFileStream upToAll: strange bug" for background and diagnosis. The problem was introduced in Multilingual-tonyg.218 and merged to trunk in Multilingual-pre.230
From: Bob Arning
Date: Sun, 21 Jan 2018 07:01:37 -0500
Subject: Re: [squeak-dev] MultiByteFileStream upToAll: strange bug
The culprit is MultiByteFileStream>>upToPosition: which was referencing the instVar <position> directly. Changing that to "self position" allows it to stop at the right place.
=============== Diff against Multilingual-pre.232 ===============
----- Method: MultiByteFileStream>>upToPosition: (in category 'accessing') -----
upToPosition: anInteger
"Answer a subcollection containing items starting from the current position and ending including the given position. Usefully different to #next: in that positions measure *bytes* from the file, where #next: wants to measure *characters*."
^self collectionSpecies new: 1000 streamContents: [ :stream |
| ch |
+ [ (ch := self next) == nil or: [ self position > anInteger ] ]
- [ (ch := self next) == nil or: [ position > anInteger ] ]
whileFalse: [ stream next
Tobias Pape
2018-01-21 21:19:11 UTC
Permalink
Post by David T. Lewis
Post by Tobias Pape
Cool!
Since Multilingual-tonyg.218 was part of a fix for Mantis#4665 (bugs.squeak.org/bug_view_advanced_page.php?bug_id=4665),
can we make sure that we don't have a regression because of that?
:)
(MultiByteFileStreamTest selector: #testUpToAllAscii) run
==> 1 run, 1 passes, 0 expected failures, 0 failures, 0 errors, 0 unexpected passes
(MultiByteFileStreamTest selector: #testUpToAllUtf) run
==> 1 run, 1 passes, 0 expected failures, 0 failures, 0 errors, 0 unexpected passes
So if you believe the unit tests, then there is no regression :-)
Great!
I just wanted to make sure nothing is lost.
Post by David T. Lewis
I am not the best person to be looking into multilingual issues, inasmuch
as my formal education ended with seven bit ASCII. It would be good if
someone else can double check this too.
Nah, I trust you :D

Best regards
-Tobias
Post by David T. Lewis
Dave
Post by Tobias Pape
Best regards
-Tobias
Post by c***@source.squeak.org
http://source.squeak.org/trunk/Multilingual-dtl.233.mcz
==================== Summary ====================
Name: Multilingual-dtl.233
Author: dtl
Time: 21 January 2018, 11:03:22.374228 am
UUID: 271b4b88-c037-4669-b2d2-15375755dcb4
Ancestors: Multilingual-pre.232
MutliByteFileStream>>upToPosition: fix provided by Bob Arning.
See squeak-dev discussion thread "MultiByteFileStream upToAll: strange bug" for background and diagnosis. The problem was introduced in Multilingual-tonyg.218 and merged to trunk in Multilingual-pre.230
From: Bob Arning
Date: Sun, 21 Jan 2018 07:01:37 -0500
Subject: Re: [squeak-dev] MultiByteFileStream upToAll: strange bug
The culprit is MultiByteFileStream>>upToPosition: which was referencing the instVar <position> directly. Changing that to "self position" allows it to stop at the right place.
=============== Diff against Multilingual-pre.232 ===============
----- Method: MultiByteFileStream>>upToPosition: (in category 'accessing') -----
upToPosition: anInteger
"Answer a subcollection containing items starting from the current position and ending including the given position. Usefully different to #next: in that positions measure *bytes* from the file, where #next: wants to measure *characters*."
^self collectionSpecies new: 1000 streamContents: [ :stream |
| ch |
+ [ (ch := self next) == nil or: [ self position > anInteger ] ]
- [ (ch := self next) == nil or: [ position > anInteger ] ]
whileFalse: [ stream nextPut: ch ] ]!
Eliot Miranda
2018-01-22 10:16:25 UTC
Permalink
Hi David,

can you add a comment to the method near the sending of position that explains? e.g. "send position so that this works with multibyte rncodings such as UTF8". Otherwise the temptation may remain to repeat the mistake.

_,,,^..^,,,_ (phone)
Post by Tobias Pape
Cool!
Since Multilingual-tonyg.218 was part of a fix for Mantis#4665 (bugs.squeak.org/bug_view_advanced_page.php?bug_id=4665),
can we make sure that we don't have a regression because of that?
:)
Best regards
-Tobias
Post by c***@source.squeak.org
http://source.squeak.org/trunk/Multilingual-dtl.233.mcz
==================== Summary ====================
Name: Multilingual-dtl.233
Author: dtl
Time: 21 January 2018, 11:03:22.374228 am
UUID: 271b4b88-c037-4669-b2d2-15375755dcb4
Ancestors: Multilingual-pre.232
MutliByteFileStream>>upToPosition: fix provided by Bob Arning.
See squeak-dev discussion thread "MultiByteFileStream upToAll: strange bug" for background and diagnosis. The problem was introduced in Multilingual-tonyg.218 and merged to trunk in Multilingual-pre.230
From: Bob Arning
Date: Sun, 21 Jan 2018 07:01:37 -0500
Subject: Re: [squeak-dev] MultiByteFileStream upToAll: strange bug
The culprit is MultiByteFileStream>>upToPosition: which was referencing the instVar <position> directly. Changing that to "self position" allows it to stop at the right place.
=============== Diff against Multilingual-pre.232 ===============
----- Method: MultiByteFileStream>>upToPosition: (in category 'accessing') -----
upToPosition: anInteger
"Answer a subcollection containing items starting from the current position and ending including the given position. Usefully different to #next: in that positions measure *bytes* from the file, where #next: wants to measure *characters*."
^self collectionSpecies new: 1000 streamContents: [ :stream |
| ch |
+ [ (ch := self next) == nil or: [ self position > anInteger ] ]
- [ (ch := self next) == nil or: [ position > anInteger ] ]
Tobias Pape
2018-01-22 10:20:34 UTC
Permalink
Post by Eliot Miranda
Hi David,
can you add a comment to the method near the sending of position that explains? e.g. "send position so that this works with multibyte rncodings such as UTF8". Otherwise the temptation may remain to repeat the mistake.
That's one reason why I always prefer accessors over direct inst-vars ;P
Best
-Tobias *tongue-in-cheek*
Post by Eliot Miranda
_,,,^..^,,,_ (phone)
Post by Tobias Pape
Cool!
Since Multilingual-tonyg.218 was part of a fix for Mantis#4665 (bugs.squeak.org/bug_view_advanced_page.php?bug_id=4665),
can we make sure that we don't have a regression because of that?
:)
Best regards
-Tobias
Post by c***@source.squeak.org
http://source.squeak.org/trunk/Multilingual-dtl.233.mcz
==================== Summary ====================
Name: Multilingual-dtl.233
Author: dtl
Time: 21 January 2018, 11:03:22.374228 am
UUID: 271b4b88-c037-4669-b2d2-15375755dcb4
Ancestors: Multilingual-pre.232
MutliByteFileStream>>upToPosition: fix provided by Bob Arning.
See squeak-dev discussion thread "MultiByteFileStream upToAll: strange bug" for background and diagnosis. The problem was introduced in Multilingual-tonyg.218 and merged to trunk in Multilingual-pre.230
From: Bob Arning
Date: Sun, 21 Jan 2018 07:01:37 -0500
Subject: Re: [squeak-dev] MultiByteFileStream upToAll: strange bug
The culprit is MultiByteFileStream>>upToPosition: which was referencing the instVar <position> directly. Changing that to "self position" allows it to stop at the right place.
=============== Diff against Multilingual-pre.232 ===============
----- Method: MultiByteFileStream>>upToPosition: (in category 'accessing') -----
upToPosition: anInteger
"Answer a subcollection containing items starting from the current position and ending including the given position. Usefully different to #next: in that positions measure *bytes* from the file, where #next: wants to measure *characters*."
^self collectionSpecies new: 1000 streamContents: [ :stream |
| ch |
+ [ (ch := self next) == nil or: [ self position > anInteger ] ]
- [ (ch := self next) == nil or: [ position > anInteger ] ]
whileFalse: [ stream nextPut: ch ] ]!
Levente Uzonyi
2018-01-22 12:44:02 UTC
Permalink
Post by Tobias Pape
Post by Eliot Miranda
Hi David,
can you add a comment to the method near the sending of position that explains? e.g. "send position so that this works with multibyte rncodings such as UTF8". Otherwise the temptation may remain to repeat the mistake.
That's one reason why I always prefer accessors over direct inst-vars ;P
The sole reason the accessor must be used in this case is the misuse of
inheritance.

Levente
Post by Tobias Pape
Best
-Tobias *tongue-in-cheek*
Post by Eliot Miranda
_,,,^..^,,,_ (phone)
Post by Tobias Pape
Cool!
Since Multilingual-tonyg.218 was part of a fix for Mantis#4665 (bugs.squeak.org/bug_view_advanced_page.php?bug_id=4665),
can we make sure that we don't have a regression because of that?
:)
Best regards
-Tobias
Post by c***@source.squeak.org
http://source.squeak.org/trunk/Multilingual-dtl.233.mcz
==================== Summary ====================
Name: Multilingual-dtl.233
Author: dtl
Time: 21 January 2018, 11:03:22.374228 am
UUID: 271b4b88-c037-4669-b2d2-15375755dcb4
Ancestors: Multilingual-pre.232
MutliByteFileStream>>upToPosition: fix provided by Bob Arning.
See squeak-dev discussion thread "MultiByteFileStream upToAll: strange bug" for background and diagnosis. The problem was introduced in Multilingual-tonyg.218 and merged to trunk in Multilingual-pre.230
From: Bob Arning
Date: Sun, 21 Jan 2018 07:01:37 -0500
Subject: Re: [squeak-dev] MultiByteFileStream upToAll: strange bug
The culprit is MultiByteFileStream>>upToPosition: which was referencing the instVar <position> directly. Changing that to "self position" allows it to stop at the right place.
=============== Diff against Multilingual-pre.232 ===============
----- Method: MultiByteFileStream>>upToPosition: (in category 'accessing') -----
upToPosition: anInteger
"Answer a subcollection containing items starting from the current position and ending including the given position. Usefully different to #next: in that positions measure *bytes* from the file, where #next: wants to measure *characters*."
^self collectionSpecies new: 1000 streamContents: [ :stream |
| ch |
+ [ (ch := self next) == nil or: [ self position > anInteger ] ]
- [ (ch := self next) == nil or: [ position > anInteger ] ]
Bob Arning
2018-01-22 13:07:54 UTC
Permalink
right - it's unfortunate that FileStream inherits from Stream because
everyone *knows* that <position> is what it says it is. Until it isn't.
Post by Levente Uzonyi
Post by Tobias Pape
   can you add a comment to the method near the sending of position
that explains?  e.g. "send position so that this works with
multibyte rncodings such as UTF8".  Otherwise the temptation may
remain to repeat the mistake.
That's one reason why I always prefer accessors over direct inst-vars ;P
The sole reason the accessor must be used in this case is the misuse
of inheritance.
David T. Lewis
2018-01-22 13:27:54 UTC
Permalink
Post by Eliot Miranda
Hi David,
can you add a comment to the method near the sending of position that explains? e.g. "send position so that this works with multibyte rncodings such as UTF8". Otherwise the temptation may remain to repeat the mistake.
There are quite a few senders of StandardFileStream>>position. The unit tests
may be a better defence in this case.

Dave
Post by Eliot Miranda
_,,,^..^,,,_ (phone)
Post by Tobias Pape
Cool!
Since Multilingual-tonyg.218 was part of a fix for Mantis#4665 (bugs.squeak.org/bug_view_advanced_page.php?bug_id=4665),
can we make sure that we don't have a regression because of that?
:)
Best regards
-Tobias
Post by c***@source.squeak.org
http://source.squeak.org/trunk/Multilingual-dtl.233.mcz
==================== Summary ====================
Name: Multilingual-dtl.233
Author: dtl
Time: 21 January 2018, 11:03:22.374228 am
UUID: 271b4b88-c037-4669-b2d2-15375755dcb4
Ancestors: Multilingual-pre.232
MutliByteFileStream>>upToPosition: fix provided by Bob Arning.
See squeak-dev discussion thread "MultiByteFileStream upToAll: strange bug" for background and diagnosis. The problem was introduced in Multilingual-tonyg.218 and merged to trunk in Multilingual-pre.230
From: Bob Arning
Date: Sun, 21 Jan 2018 07:01:37 -0500
Subject: Re: [squeak-dev] MultiByteFileStream upToAll: strange bug
The culprit is MultiByteFileStream>>upToPosition: which was referencing the instVar <position> directly. Changing that to "self position" allows it to stop at the right place.
=============== Diff against Multilingual-pre.232 ===============
----- Method: MultiByteFileStream>>upToPosition: (in category 'accessing') -----
upToPosition: anInteger
"Answer a subcollection containing items starting from the current position and ending including the given position. Usefully different to #next: in that positions measure *bytes* from the file, where #next: wants to measure *characters*."
^self collectionSpecies new: 1000 streamContents: [ :stream |
| ch |
+ [ (ch := self next) == nil or: [ self position > anInteger ] ]
- [ (ch := self next) == nil or: [ position > anInteger ] ]
whileFalse: [ stream nextPut: ch ] ]!
Loading...