[squeak-dev] The Inbox: Kernel-ct.1455.mcz

Thiede, Christoph Christoph.Thiede at student.hpi.uni-potsdam.de
Tue Apr 5 11:48:47 UTC 2022


Hi Marcel,


thanks for the feedback.


> I mean, we don't store sources in a method trailer but don't log them at all if that preference is enabled.


I wish we did. It should be pretty easy, see CompiledMethod>>#getSourceFor:in::


aCompiledMethod properties at: #source put: aStringOrText.


What do you think? :-)



> I think there should be a very prominent indication in the Morphic world whenever the image is configured to not log changes.


As this is an optional preference, I'm not sure whether we need that indication. But one option could be appending a text like ' (no changes)' to the changes menu item in the docking bar.


Best,

Christoph

________________________________
Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von Taeumel, Marcel
Gesendet: Dienstag, 5. April 2022 11:34:16
An: squeak-dev
Betreff: Re: [squeak-dev] The Inbox: Kernel-ct.1455.mcz

At this level, RemoteString, String, and Text should be interchangeable. At least to the tools that query that information. I wouldn't worry. Looks good.

Well, it's still kind of different from the Decompiler approach, right? I mean, we don't store sources in a method trailer but don't log them at all if that preference is enabled. Then again, string literals in methods are literals and live in the image anyway. So... :-)

Hmm... I think there should be a very prominent indication in the Morphic world whenever the image is configured to not log changes. Any ideas?

Best,
Marcel

Am 04.04.2022 13:45:36 schrieb christoph.thiede at student.hpi.uni-potsdam.de <christoph.thiede at student.hpi.uni-potsdam.de>:

The probably largest change is that the instvar classComment is no longer guaranteed to be either nil or a RemoteString, but can also be a String or Text. I hope I haven't missed any other user. Otherwise, I'd be happy to merge this.

Best,
Christoph

---
Sent from Squeak Inbox Talk<https://github.com/hpi-swa-lab/squeak-inbox-talk>

On 2022-04-02T10:21:26-07:00, tim at rowledge.org wrote:

> Looks pretty good to me; I must confess I didn't think of class comments when I added the original "save no source" thing.
>
> > On 2022-04-02, at 4:04 PM, commits at source.squeak.org wrote:
> >
> > A new version of Kernel was added to project The Inbox:
> > http://source.squeak.org/inbox/Kernel-ct.1455.mcz
> >
> > ==================== Summary ====================
> >
> > Name: Kernel-ct.1455
> > Author: ct
> > Time: 2 April 2022, 6:03:46.669052 pm
> > UUID: d86c9223-8db7-e541-96bd-cde0c9b7fb92
> > Ancestors: Kernel-ul.1454
> >
> > Proposal: Adds basic support for #acceptsLoggingOfCompilation to class comments by allowing strings in BasicClassOrganizer.classComment. With this change, installing updates in a read-only image becomes more feasible. Please review.
> >
> > =============== Diff against Kernel-ul.1454 ===============
> >
> > Item was changed:
> > ----- Method: BasicClassOrganizer>>classComment (in category 'accessing') -----
> > classComment
> >     classComment
> >         ifNil: [^ ''].
> > +     (classComment isString
> > +             or: [classComment isText])
> > +         ifTrue: [^ classComment].
> > +     ^ classComment text
> > +         ifNil: ['']!
> > -     ^ classComment text ifNil: ['']!
> >
> > Item was changed:
> > ----- Method: BasicClassOrganizer>>classComment:stamp: (in category 'accessing') -----
> > classComment: aString stamp: aStamp
> > +     "Store the comment, aString, associated with the object that refers to the receiver. PRIVATE!! Clients should use aClass classComment: instead."
> > -     "Store the comment, aString, associated with the object that refers to the receiver."
> >
> >     self commentStamp: aStamp.
> >     (aString isKindOf: RemoteString)
> >         ifTrue: [classComment := aString]
> >         ifFalse: [(aString == nil or: [aString size = 0])
> >             ifTrue: [classComment := nil]
> >             ifFalse:
> > +                 [classComment := aString]]
> > -                 [self error: 'use aClass classComment:'.
> > -                 classComment := RemoteString newString: aString onFileNumber: 2]]
> >                 "Later add priorSource and date and initials?"!
> >
> > Item was changed:
> > ----- Method: BasicClassOrganizer>>commentRemoteStr (in category 'accessing') -----
> > commentRemoteStr
> > +     ^ (classComment isKindOf: RemoteString)
> > +         ifTrue: [classComment]!
> > -     ^ classComment!
> >
> > Item was changed:
> > ----- Method: BasicClassOrganizer>>fileOutCommentOn:moveSource:toFile: (in category 'fileIn/Out') -----
> > fileOutCommentOn: aFileStream moveSource: moveSource toFile: fileIndex
> >     "Copy the class comment to aFileStream. If moveSource is true (as in compressChanges or compressSources, then update classComment to point to the new file."
> >     | fileComment |
> > +     self hasNoComment ifTrue: [^ self].
> > +
> > +     aFileStream cr.
> > +     fileComment := RemoteString
> > +         newString: self classComment
> > +         onFileNumber: fileIndex
> > +         toFile: aFileStream.
> > +     moveSource ifTrue: [classComment := fileComment].!
> > -     classComment ifNotNil:
> > -             [aFileStream cr.
> > -             fileComment := RemoteString newString: classComment text
> > -                             onFileNumber: fileIndex toFile: aFileStream.
> > -             moveSource ifTrue: [classComment := fileComment]]!
> >
> > Item was changed:
> > ----- Method: BasicClassOrganizer>>moveChangedCommentToFile:numbered: (in category 'fileIn/Out') -----
> > moveChangedCommentToFile: aFileStream numbered: fileIndex
> >     "If the comment is in the changes file, then move it to a new file."
> >
> > +     (self commentRemoteStr ~~ nil and: [classComment sourceFileNumber > 1]) ifTrue:
> > -     (classComment ~~ nil and: [classComment sourceFileNumber > 1]) ifTrue:
> >         [self fileOutCommentOn: aFileStream moveSource: true toFile: fileIndex]!
> >
> > Item was changed:
> > ----- Method: ClassDescription>>classComment:stamp: (in category 'fileIn/Out') -----
> > classComment: aString stamp: aStamp
> >     "Store the comment, aString or Text or RemoteString, associated with the class we are organizing. Empty string gets stored only if had a non-empty one before."
> >
> > -     | ptr header file oldCommentRemoteStr |
> >     (aString isKindOf: RemoteString) ifTrue:
> >         [SystemChangeNotifier uniqueInstance classCommented: self.
> >         ^ self organization classComment: aString stamp: aStamp].
> >
> > +     self acceptsLoggingOfCompilation
> > +         ifTrue:
> > +             [| ptr header file oldCommentRemoteStr |
> > +             oldCommentRemoteStr := self organization commentRemoteStr.
> > +             (aString size = 0) & (oldCommentRemoteStr == nil) ifTrue:
> > +                 ["never had a class comment, no need to write empty string out"
> > +                 ^ self organization classComment: nil].
> > +
> > +             ptr := oldCommentRemoteStr ifNil: [0] ifNotNil: [oldCommentRemoteStr sourcePointer].
> > +             SourceFiles ifNotNil: [(file := SourceFiles at: 2) ifNotNil:
> > +                 [file setToEnd; cr; nextPut: $!!.    "directly"
> > +                 "Should be saying (file command: ''H3'') for HTML, but ignoring it here"
> > +                 header := String streamContents: [:strm | strm nextPutAll: self name;
> > +                     nextPutAll: ' commentStamp: '.
> > +                     aStamp storeOn: strm.
> > +                     strm nextPutAll: ' prior: '; nextPutAll: ptr printString].
> > +                 file nextChunkPut: header]].
> > +             self organization classComment: (RemoteString newString: aString onFileNumber: 2) stamp: aStamp.
> > +             file ifNotNil: [ InMidstOfFileinNotification signal ifFalse: [ file flush ] ]]
> > +         ifFalse:
> > +             [self organization classComment: aString stamp: aStamp].
> > +
> > +     SystemChangeNotifier uniqueInstance classCommented: self.!
> > -     oldCommentRemoteStr := self organization commentRemoteStr.
> > -     (aString size = 0) & (oldCommentRemoteStr == nil) ifTrue: [^ self organization classComment: nil].
> > -         "never had a class comment, no need to write empty string out"
> > -
> > -     ptr := oldCommentRemoteStr ifNil: [0] ifNotNil: [oldCommentRemoteStr sourcePointer].
> > -     SourceFiles ifNotNil: [(file := SourceFiles at: 2) ifNotNil:
> > -         [file setToEnd; cr; nextPut: $!!.    "directly"
> > -         "Should be saying (file command: 'H3') for HTML, but ignoring it here"
> > -         header := String streamContents: [:strm | strm nextPutAll: self name;
> > -             nextPutAll: ' commentStamp: '.
> > -             aStamp storeOn: strm.
> > -             strm nextPutAll: ' prior: '; nextPutAll: ptr printString].
> > -         file nextChunkPut: header]].
> > -     self organization classComment: (RemoteString newString: aString onFileNumber: 2) stamp: aStamp.
> > -     file ifNotNil: [ InMidstOfFileinNotification signal ifFalse: [ file flush ] ].
> > -     SystemChangeNotifier uniqueInstance classCommented: self.
> > - !
> >
> >
> >
>
>
> tim
> --
> tim Rowledge; tim at rowledge.org; http://www.rowledge.org/tim
> Oxymorons: Exact estimate
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220405/67bdac84/attachment.html>


More information about the Squeak-dev mailing list