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: [hidden email] 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 nextPut: ch ] ]! |
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 > On 21.01.2018, at 17:03, [hidden email] wrote: > > 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: [hidden email] > 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 nextPut: ch ] ]! > > |
On Sun, Jan 21, 2018 at 07:48:31PM +0100, Tobias Pape wrote:
> 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 > Best regards > -Tobias > > On 21.01.2018, at 17:03, [hidden email] wrote: > > > > 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: [hidden email] > > 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 nextPut: ch ] ]! > > > > > > |
> On 21.01.2018, at 20:10, David T. Lewis <[hidden email]> wrote: > > On Sun, Jan 21, 2018 at 07:48:31PM +0100, Tobias Pape wrote: >> 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 :-) Great! I just wanted to make sure nothing is lost. > > 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 > > Dave > > >> Best regards >> -Tobias >>> On 21.01.2018, at 17:03, [hidden email] wrote: >>> >>> 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: [hidden email] >>> 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 nextPut: ch ] ]! >>> >>> >> >> > |
In reply to this post by Tobias Pape
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) > On Jan 21, 2018, at 10:48 AM, Tobias Pape <[hidden email]> wrote: > > 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 >> On 21.01.2018, at 17:03, [hidden email] wrote: >> >> 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: [hidden email] >> 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 nextPut: ch ] ]! >> >> > > |
> On 22.01.2018, at 11:16, Eliot Miranda <[hidden email]> wrote: > > 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* > > _,,,^..^,,,_ (phone) > >> On Jan 21, 2018, at 10:48 AM, Tobias Pape <[hidden email]> wrote: >> >> 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 >>> On 21.01.2018, at 17:03, [hidden email] wrote: >>> >>> 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: [hidden email] >>> 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 nextPut: ch ] ]! >>> >>> >> >> > |
On Mon, 22 Jan 2018, Tobias Pape wrote:
> >> On 22.01.2018, at 11:16, Eliot Miranda <[hidden email]> wrote: >> >> 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 > Best > -Tobias *tongue-in-cheek* > >> >> _,,,^..^,,,_ (phone) >> >>> On Jan 21, 2018, at 10:48 AM, Tobias Pape <[hidden email]> wrote: >>> >>> 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 >>>> On 21.01.2018, at 17:03, [hidden email] wrote: >>>> >>>> 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: [hidden email] >>>> 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 nextPut: ch ] ]! >>>> >>>> >>> >>> >> |
right - it's unfortunate that FileStream
inherits from Stream because everyone *knows* that
<position> is what it says it is. Until it isn't. On 1/22/18 7:44 AM, Levente Uzonyi
wrote:
|
In reply to this post by Eliot Miranda-2
On Mon, Jan 22, 2018 at 02:16:25AM -0800, Eliot Miranda wrote:
> 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 > _,,,^..^,,,_ (phone) > > > On Jan 21, 2018, at 10:48 AM, Tobias Pape <[hidden email]> wrote: > > > > 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 > >> On 21.01.2018, at 17:03, [hidden email] wrote: > >> > >> 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: [hidden email] > >> 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 nextPut: ch ] ]! > >> > >> > > > > > |
Free forum by Nabble | Edit this page |