Hi Marcel,


thanks for the feedback! I'm currently continuing the refactoring of this method anyway, so this version can be moved to the treated inbox of course.


Yes, I think #asCommaStringAnd should be fixed (even though the oxford comma appears not to be the universal truth because, in certain scenarios, it can also add ambiguity - see https://en.wikipedia.org/wiki/Serial_comma#Creating_ambiguity, but it is being widely accepted). Will upload a patch right now. :-)


In general, I don't think that adding more characters to the annotation bar is a good idea


Fair points. Will return to the single comma-separation in the next upload.


Please note that the "and" was not the only part of the output beautification. I also removed the trailing separator character. :-)


Best,

Christoph


Von: Squeak-dev <squeak-dev-bounces@lists.squeakfoundation.org> im Auftrag von Taeumel, Marcel
Gesendet: Donnerstag, 11. März 2021 09:04:08
An: squeak-dev
Betreff: Re: [squeak-dev] The Inbox: Tools-ct.1015.mcz
 
Hi there :-)

-0.5 for the output beautification
+1 for the rest

If you would like to add commas to the text, please also put one before the "and": https://www.grammarly.com/blog/what-is-the-oxford-comma-and-why-do-people-care-so-much-about-it/

In general, I don't think that adding more characters to the annotation bar is a good idea since its space is restricted so much already. Also, maybe a simpler representation would promote do-it scripting? :-) Such as through #splitBy:, which becomes more complicated with "," and "and".

Best,
Marcel

Am 01.11.2020 14:58:58 schrieb commits@source.squeak.org <commits@source.squeak.org>:

Christoph Thiede uploaded a new version of Tools to project The Inbox:
http://source.squeak.org/inbox/Tools-ct.1015.mcz

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

Name: Tools-ct.1015
Author: ct
Time: 1 November 2020, 3:58:41.62325 pm
UUID: 0f1723d1-90c5-ca4e-88ad-56f6800b2b17
Ancestors: Tools-tpr.1014

Beautify CodeHolder >> #annotationForSelector:ofClass: implementation and output. Remove dangling separator character and add multilingual support.

Example - before:
mt 2/13/2020 13:49 · running · 2 implementors · in change sets: Morphic-pre.1466 Morphic-mt.1507 Morphic-mt.1607 Morphic-cmm.1617 Morphic-mt.1622 ·
Example - after:
mt 2/13/2020 13:49 · running · 2 implementors · in change sets: Morphic-pre.1466, Morphic-mt.1507, Morphic-mt.1607, Morphic-cmm.1617 and Morphic-mt.1622

=============== Diff against Tools-tpr.1014 ===============

Item was changed:
----- Method: CodeHolder>>annotationForSelector:ofClass: (in category 'annotation') -----
annotationForSelector: aSelector ofClass: aClass
"Provide a line of content for an annotation pane, representing
information about the given selector and class"
| separator aStream requestList |
aSelector == #Comment
ifTrue: [^ self annotationForClassCommentFor: aClass].
aSelector == #Definition
ifTrue: [^ self annotationForClassDefinitionFor: aClass].
aSelector == #Hierarchy
ifTrue: [^ self annotationForHierarchyFor: aClass].
aStream := (String new: 512) writeStream.
requestList := self annotationRequests.
+ separator := self annotationSeparator.
- separator := requestList size > 1
- ifTrue: [self annotationSeparator]
- ifFalse: [''].
requestList
do: [:aRequest | | aString sendersCount aComment aCategory implementorsCount aList stamp authorInitials |
aRequest == #firstComment
ifTrue: [aComment := aClass firstCommentAt: aSelector.
aComment isEmptyOrNil
+ ifFalse: [aStream nextPutAll: aComment]].
- ifFalse: [aStream nextPutAll: aComment , separator]].
aRequest == #masterComment
ifTrue: [aComment := aClass supermostPrecodeCommentFor: aSelector.
aComment isEmptyOrNil
+ ifFalse: [aStream nextPutAll: aComment]].
- ifFalse: [aStream nextPutAll: aComment , separator]].
aRequest == #documentation
ifTrue: [aComment := aClass precodeCommentOrInheritedCommentFor: aSelector.
aComment isEmptyOrNil
+ ifFalse: [aStream nextPutAll: aComment]].
- ifFalse: [aStream nextPutAll: aComment , separator]].
aRequest == #timeStamp
ifTrue: [stamp := self timeStamp.
aStream
+ nextPutAll: (stamp ifEmpty: ['no timeStamp' translated])].
- nextPutAll: (stamp size > 0
- ifTrue: [stamp , separator]
- ifFalse: ['no timeStamp' , separator])].
aRequest == #author
ifTrue: [authorInitials := self timeStamp
findTokens ifEmpty: [''] ifNotEmpty: [:tokens | tokens first].
aStream
nextPutAll: (SystemNavigation authorsInverted
at: authorInitials
ifPresent: [:fullNames | fullNames anyOne]
ifAbsent: ['unknown author']), separator].
aRequest == #messageCategory
ifTrue: [aCategory := aClass organization categoryOfElement: aSelector.
aCategory
ifNotNil: ["woud be nil for a method no longer present,
e.g. in a recent-submissions browser"
+ aStream nextPutAll: aCategory]].
- aStream nextPutAll: aCategory , separator]].
aRequest == #sendersCount
ifTrue: [sendersCount := (self systemNavigation allCallsOn: aSelector) size.
sendersCount := sendersCount = 1
+ ifTrue: ['1 sender' translated]
+ ifFalse: ['{1} senders' translated format: {sendersCount}].
+ aStream nextPutAll: sendersCount].
- ifTrue: ['1 sender']
- ifFalse: [sendersCount printString , ' senders'].
- aStream nextPutAll: sendersCount , separator].
aRequest == #implementorsCount
ifTrue: [implementorsCount := self systemNavigation numberOfImplementorsOf: aSelector.
implementorsCount := implementorsCount = 1
+ ifTrue: ['1 implementor' translated]
+ ifFalse: ['{1} implementors' translated format: {implementorsCount}].
+ aStream nextPutAll: implementorsCount].
- ifTrue: ['1 implementor']
- ifFalse: [implementorsCount printString , ' implementors'].
- aStream nextPutAll: implementorsCount , separator].
aRequest == #priorVersionsCount
ifTrue: [self
addPriorVersionsCountForSelector: aSelector
ofClass: aClass
to: aStream].
aRequest == #priorTimeStamp
ifTrue: [stamp := VersionsBrowser
timeStampFor: aSelector
class: aClass
reverseOrdinal: 2.
stamp
+ ifNotNil: [aStream nextPutAll: 'prior time stamp: ' translated , stamp]].
- ifNotNil: [aStream nextPutAll: 'prior time stamp: ' , stamp , separator]].
aRequest == #recentChangeSet
ifTrue: [aString := ChangesOrganizer mostRecentChangeSetWithChangeForClass: aClass selector: aSelector.
+ aStream nextPutAll: aString].
- aString size > 0
- ifTrue: [aStream nextPutAll: aString , separator]].
aRequest == #allChangeSets
ifTrue: [aList := ChangesOrganizer allChangeSetsWithClass: aClass selector: aSelector.
+ aList
+ ifNotEmpty: [aList size = 1
+ ifTrue: [aStream nextPutAll: 'only in change set ' translated]
+ ifFalse: [aStream nextPutAll: 'in change sets: ' translated].
+ aStream nextPutAll: (aList collect:
+ [:changeSet | changeSet name]) asCommaStringAnd]
+ ifEmpty: [aStream nextPutAll: 'in no change set' translated]]]
+ separatedBy: [
+ aStream nextPutAll: separator].
- aList size > 0
- ifTrue: [aList size = 1
- ifTrue: [aStream nextPutAll: 'only in change set ']
- ifFalse: [aStream nextPutAll: 'in change sets: '].
- aList
- do: [:aChangeSet | aStream nextPutAll: aChangeSet name , ' ']]
- ifFalse: [aStream nextPutAll: 'in no change set'].
- aStream nextPutAll: separator]].
^ aStream contents!