[squeak-dev] The Inbox: Tools-ct.1054.mcz

Tom Beckmann tomjonabc at gmail.com
Fri May 7 05:35:02 UTC 2021


Hi Chris,

speaking from experience with an extension like this: I started out with a
script that reformatted all methods in my package (it was a good idea) and
moved on to using something like this proposed extension.

For further context, I have gotten used to and comfortable with the idea
that formatting is just busy work in 95% of cases that I'd like to spend on
something productive rather than moving whitespace.

Undoubtedly, using a pretty printer on most trunk code is infeasible, as
each method/class/package currently follows different intricacies of
secondary notation. Additionally, there are of course some "special" (from
the POV of our pretty printer) formatting choices that authors deliberately
chose to make a point about the code. This type of secondary notation,
where it's actually valuable, is I think quite common in trunk code, but
exceedingly uncommon in code I produce in the daily business.
I don't think anyone currently even considers applying a pretty printer
against all trunk code, for various reasons.

Since it's a preference I would give the proposed change a +1. It supports
a valuable workflow that I believe is slowly becoming feasible in Squeak.
The Ctrl+Shift+S shortcut could even be inverted when the preference is
active so that you can keep formatting idiosyncrasies where it's
appropriate.

It may be important to note that we are working on having a pretty printer
understand common Smalltalk idioms and format those accordingly. We are
also planning to try and maintain deliberate choices, such as empty lines,
strides in array formatting, or comment positions.

If you've never tried programming in an ecosystem where there's a
well-accepted standard for code style that can be automatically applied,
I'd recommend you give it a shot. At least for me, it allowed performing
changes more directly (no tedious cleanup each time I want to look at an
intermediate or final state of a change) and saved a good chunk of brain
power that I could invest elsewhere :)

Best,
Tom

On Fri, May 7, 2021, 02:25 Chris Muller <asqueaker at gmail.com> wrote:

> -1.  The IDE should not break the boundaries of roles between the
> human and IDE.  IOW, it should maintain explicit gesture separation
> between what the human crafted, and what is stored in the system.  If
> this was really a good idea, why not write a script to simply format
> all methods in the whole system? (answer: because I'm sure you agree
> that's a bad idea).  Or why not just use #browseWithPrettyPrint?
>
> There is already a hot-key for pretty-print (Shift+Cmd+S), so you can
> obtain the same effect with virtually no extra effort if you want to.
>
>
>
>
> On Thu, May 6, 2021 at 4:40 PM <commits at source.squeak.org> wrote:
> >
> > A new version of Tools was added to project The Inbox:
> > http://source.squeak.org/inbox/Tools-ct.1054.mcz
> >
> > ==================== Summary ====================
> >
> > Name: Tools-ct.1054
> > Author: ct
> > Time: 6 May 2021, 11:40:27.54561 pm
> > UUID: edc189dc-7bb9-974a-9aa3-4760e7e67239
> > Ancestors: Tools-mt.1053
> >
> > Proposal: Adds a new preference #acceptWithPrettyPrint that, if enabled,
> automatically pretty-prints every message before accepting it in a code
> holder. When used together with the preferences #browseWithPrettyPrint (and
> maybe also #diffsWithPrettyPrint), given a good pretty-printer such as
> PoppyPrint, this has the potential to make your journey through Squeak even
> prettier. :-)
> >
> > =============== Diff against Tools-mt.1053 ===============
> >
> > Item was changed:
> >   ----- Method: Browser>>defineMessageFrom:notifying: (in category
> 'message functions') -----
> >   defineMessageFrom: aString notifying: aController
> >         "Compile the expressions in aString. Notify aController if a
> syntax error occurs. Install the compiled method in the selected class
> classified under  the currently selected message category name. Answer the
> selector obtained if compilation succeeds, nil otherwise."
> >         | selectedMessageName selector category oldMessageList
> selectedClassOrMetaClass |
> >         selectedMessageName := self selectedMessageName.
> >         oldMessageList := self messageList.
> >         selectedClassOrMetaClass := self selectedClassOrMetaClass.
> >         contents := nil.
> >         selector := (selectedClassOrMetaClass newParser parseSelector:
> aString).
> >         (self metaClassIndicated
> >                 and: [(selectedClassOrMetaClass includesSelector:
> selector) not
> >                 and: [Metaclass isScarySelector: selector]])
> >                 ifTrue: ["A frist-time definition overlaps the protocol
> of Metaclasses"
> >                                 (self confirm: ((selector , ' is used in
> the existing class system.
> >   Overriding it could cause serious problems.
> >   Is this really what you want to do?') asText makeBoldFrom: 1 to:
> selector size))
> >                                 ifFalse: [^nil]].
> >         category := selectedMessageName
> >                 ifNil: [ self selectedMessageCategoryName ]
> >                 ifNotNil: [ (selectedClassOrMetaClass >>
> selectedMessageName) methodReference ifNotNil: [ : ref | ref category ]].
> > +       selector := self
> > +               basicCompile: aString
> > +               in: selectedClassOrMetaClass
> > +               classified: category
> > +               notifying: aController.
> > -       selector := selectedClassOrMetaClass
> > -                               compile: aString
> > -                               classified: category
> > -                               notifying: aController.
> >         selector == nil ifTrue: [^ nil].
> >         contents := aString copy.
> >
> >         self changed: #messageCategoryList. "Because the 'as yet
> unclassified' might just appear."
> >         self changed: #messageList. "Because we have code-dependent list
> formatting by now such as #isDeprecated."
> >
> >         selector ~~ selectedMessageName
> >                 ifTrue:
> >                         [category = ClassOrganizer nullCategory
> >                                 ifTrue: [self changed:
> #classSelectionChanged.
> >                                                 self changed: #classList.
> >                                                 self
> messageCategoryListIndex: 1].
> >                         self setClassOrganizer.  "In case organization
> not cached"
> >                         (oldMessageList includes: selector)
> >                                 ifFalse: [self changed: #messageList].
> >                         self messageListIndex: (self messageList
> indexOf: selector)].
> >         ^ selector!
> >
> > Item was added:
> > + ----- Method: CodeHolder>>basicCompile:in:classified:notifying: (in
> category 'code pane') -----
> > + basicCompile: aString in: aClassOrMetaClass classified: category
> notifying: requestor
> > +
> > +       | source |
> > +       source := SystemBrowser acceptWithPrettyPrint
> > +               ifTrue: [aClassOrMetaClass prettyPrinterClass
> > +                       format: aString in: aClassOrMetaClass notifying:
> requestor]
> > +               ifFalse: [aString].
> > +       ^ aClassOrMetaClass
> > +               compile: source
> > +               classified: category
> > +               notifying: requestor!
> >
> > Item was changed:
> >   ----- Method: CodeHolder>>compileMessage:notifying: (in category 'code
> pane') -----
> >   compileMessage: aString notifying: aController
> >         "Compile the code that was accepted by the user, placing the
> compiled method into an appropriate message category.  Return true if the
> compilation succeeded, else false."
> >
> >         | selectedMessageName selector category selectedClassOrMetaClass
> |
> >         selectedMessageName := self selectedMessageName.
> >         selectedClassOrMetaClass := self selectedClassOrMetaClass.
> >         contents := nil.
> >         selector := (selectedClassOrMetaClass newParser parseSelector:
> aString).
> >         (self metaClassIndicated
> >                 and: [(selectedClassOrMetaClass includesSelector:
> selector) not
> >                 and: [Metaclass isScarySelector: selector]])
> >                 ifTrue: ["A frist-time definition overlaps the protocol
> of Metaclasses"
> >                                 (self confirm: ((selector , ' is used in
> the existing class system.
> >   Overriding it could cause serious problems.
> >   Is this really what you want to do?') asText makeBoldFrom: 1 to:
> selector size))
> >                                 ifFalse: [^nil]].
> >         category := self selectedMessageCategoryName.
> > +       selector := self
> > +               basicCompile: aString
> > +               in: selectedClassOrMetaClass
> > +               classified: category
> > +               notifying: aController.
> > -       selector := selectedClassOrMetaClass
> > -                               compile: aString
> > -                               classified: category
> > -                               notifying: aController.
> >         selector == nil ifTrue: [^ nil].
> >         contents := aString copy.
> >         currentCompiledMethod := selectedClassOrMetaClass
> compiledMethodAt: selector.
> >         ^ true!
> >
> > Item was changed:
> >   ----- Method: DependencyBrowser>>defineMessageFrom:notifying: (in
> category 'contents') -----
> >   defineMessageFrom: aString notifying: aController
> >         "Compile the expressions in aString. Notify aController if a
> syntax error occurs. Install the compiled method in the selected class
> classified under  the currently selected message category name. Answer the
> selector obtained if compilation succeeds, nil otherwise."
> >         | selectedMessageName selector category oldMessageList |
> >         selectedMessageName := self selectedMessageName.
> >         oldMessageList := self messageList.
> >         contents := nil.
> >         selector := (self selectedClassOrMetaClass newParser
> parseSelector: aString).
> > +       selector := self
> > +               basicCompile: aString
> > +               in: self selectedClassOrMetaClass
> > +               classified: (category := self
> selectedMessageCategoryName)
> > +               notifying: aController.
> > -       selector := self selectedClassOrMetaClass
> > -                               compile: aString
> > -                               classified: (category := self
> selectedMessageCategoryName)
> > -                               notifying: aController.
> >         selector == nil ifTrue: [^ false].
> >         contents := aString copy.
> >         ^ true
> >   !
> >
> > Item was changed:
> >   AppRegistry subclass: #SystemBrowser
> >         instanceVariableNames: ''
> > +       classVariableNames: 'AcceptWithPrettyPrint BrowseWithDragNDrop
> BrowseWithPrettyPrint'
> > -       classVariableNames: 'BrowseWithDragNDrop BrowseWithPrettyPrint'
> >         poolDictionaries: ''
> >         category: 'Tools-Base'!
> >
> >   !SystemBrowser commentStamp: '<historical>' prior: 0!
> >   This is the AppRegistry class for class browsing!
> >
> > Item was added:
> > + ----- Method: SystemBrowser class>>acceptWithPrettyPrint (in category
> 'preferences') -----
> > + acceptWithPrettyPrint
> > +       <preference: 'Accept with pretty-print' category: 'browsing'
> description: 'If true, browsers will automatically pretty-print every
> method when you accept it.' type: #Boolean>
> > +       ^ AcceptWithPrettyPrint ifNil: [false].!
> >
> > Item was added:
> > + ----- Method: SystemBrowser class>>acceptWithPrettyPrint: (in category
> 'preferences') -----
> > + acceptWithPrettyPrint: aBoolean
> > +       AcceptWithPrettyPrint := aBoolean.!
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20210507/566e1f8b/attachment.html>


More information about the Squeak-dev mailing list