<div dir="auto">Hi Chris,<div dir="auto"><br></div><div dir="auto">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.</div><div dir="auto"><br></div><div dir="auto">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.</div><div dir="auto"><br></div><div dir="auto">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) <span style="font-family:sans-serif">formatting choices</span> 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.</div><div dir="auto">I don't think anyone currently even considers applying a pretty printer against all trunk code, for various reasons.</div><div dir="auto"><br></div><div dir="auto">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.</div><div dir="auto"><br></div><div dir="auto">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.</div><div dir="auto"><br></div><div dir="auto">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 :) </div><div dir="auto"><br></div><div dir="auto">Best,</div><div dir="auto">Tom</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, May 7, 2021, 02:25 Chris Muller <<a href="mailto:asqueaker@gmail.com">asqueaker@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">-1. The IDE should not break the boundaries of roles between the<br>
human and IDE. IOW, it should maintain explicit gesture separation<br>
between what the human crafted, and what is stored in the system. If<br>
this was really a good idea, why not write a script to simply format<br>
all methods in the whole system? (answer: because I'm sure you agree<br>
that's a bad idea). Or why not just use #browseWithPrettyPrint?<br>
<br>
There is already a hot-key for pretty-print (Shift+Cmd+S), so you can<br>
obtain the same effect with virtually no extra effort if you want to.<br>
<br>
<br>
<br>
<br>
On Thu, May 6, 2021 at 4:40 PM <<a href="mailto:commits@source.squeak.org" target="_blank" rel="noreferrer">commits@source.squeak.org</a>> wrote:<br>
><br>
> A new version of Tools was added to project The Inbox:<br>
> <a href="http://source.squeak.org/inbox/Tools-ct.1054.mcz" rel="noreferrer noreferrer" target="_blank">http://source.squeak.org/inbox/Tools-ct.1054.mcz</a><br>
><br>
> ==================== Summary ====================<br>
><br>
> Name: Tools-ct.1054<br>
> Author: ct<br>
> Time: 6 May 2021, 11:40:27.54561 pm<br>
> UUID: edc189dc-7bb9-974a-9aa3-4760e7e67239<br>
> Ancestors: Tools-mt.1053<br>
><br>
> 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. :-)<br>
><br>
> =============== Diff against Tools-mt.1053 ===============<br>
><br>
> Item was changed:<br>
> ----- Method: Browser>>defineMessageFrom:notifying: (in category 'message functions') -----<br>
> defineMessageFrom: aString notifying: aController<br>
> "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."<br>
> | selectedMessageName selector category oldMessageList selectedClassOrMetaClass |<br>
> selectedMessageName := self selectedMessageName.<br>
> oldMessageList := self messageList.<br>
> selectedClassOrMetaClass := self selectedClassOrMetaClass.<br>
> contents := nil.<br>
> selector := (selectedClassOrMetaClass newParser parseSelector: aString).<br>
> (self metaClassIndicated<br>
> and: [(selectedClassOrMetaClass includesSelector: selector) not<br>
> and: [Metaclass isScarySelector: selector]])<br>
> ifTrue: ["A frist-time definition overlaps the protocol of Metaclasses"<br>
> (self confirm: ((selector , ' is used in the existing class system.<br>
> Overriding it could cause serious problems.<br>
> Is this really what you want to do?') asText makeBoldFrom: 1 to: selector size))<br>
> ifFalse: [^nil]].<br>
> category := selectedMessageName<br>
> ifNil: [ self selectedMessageCategoryName ]<br>
> ifNotNil: [ (selectedClassOrMetaClass >> selectedMessageName) methodReference ifNotNil: [ : ref | ref category ]].<br>
> + selector := self<br>
> + basicCompile: aString<br>
> + in: selectedClassOrMetaClass<br>
> + classified: category<br>
> + notifying: aController.<br>
> - selector := selectedClassOrMetaClass<br>
> - compile: aString<br>
> - classified: category<br>
> - notifying: aController.<br>
> selector == nil ifTrue: [^ nil].<br>
> contents := aString copy.<br>
><br>
> self changed: #messageCategoryList. "Because the 'as yet unclassified' might just appear."<br>
> self changed: #messageList. "Because we have code-dependent list formatting by now such as #isDeprecated."<br>
><br>
> selector ~~ selectedMessageName<br>
> ifTrue:<br>
> [category = ClassOrganizer nullCategory<br>
> ifTrue: [self changed: #classSelectionChanged.<br>
> self changed: #classList.<br>
> self messageCategoryListIndex: 1].<br>
> self setClassOrganizer. "In case organization not cached"<br>
> (oldMessageList includes: selector)<br>
> ifFalse: [self changed: #messageList].<br>
> self messageListIndex: (self messageList indexOf: selector)].<br>
> ^ selector!<br>
><br>
> Item was added:<br>
> + ----- Method: CodeHolder>>basicCompile:in:classified:notifying: (in category 'code pane') -----<br>
> + basicCompile: aString in: aClassOrMetaClass classified: category notifying: requestor<br>
> +<br>
> + | source |<br>
> + source := SystemBrowser acceptWithPrettyPrint<br>
> + ifTrue: [aClassOrMetaClass prettyPrinterClass<br>
> + format: aString in: aClassOrMetaClass notifying: requestor]<br>
> + ifFalse: [aString].<br>
> + ^ aClassOrMetaClass<br>
> + compile: source<br>
> + classified: category<br>
> + notifying: requestor!<br>
><br>
> Item was changed:<br>
> ----- Method: CodeHolder>>compileMessage:notifying: (in category 'code pane') -----<br>
> compileMessage: aString notifying: aController<br>
> "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."<br>
><br>
> | selectedMessageName selector category selectedClassOrMetaClass |<br>
> selectedMessageName := self selectedMessageName.<br>
> selectedClassOrMetaClass := self selectedClassOrMetaClass.<br>
> contents := nil.<br>
> selector := (selectedClassOrMetaClass newParser parseSelector: aString).<br>
> (self metaClassIndicated<br>
> and: [(selectedClassOrMetaClass includesSelector: selector) not<br>
> and: [Metaclass isScarySelector: selector]])<br>
> ifTrue: ["A frist-time definition overlaps the protocol of Metaclasses"<br>
> (self confirm: ((selector , ' is used in the existing class system.<br>
> Overriding it could cause serious problems.<br>
> Is this really what you want to do?') asText makeBoldFrom: 1 to: selector size))<br>
> ifFalse: [^nil]].<br>
> category := self selectedMessageCategoryName.<br>
> + selector := self<br>
> + basicCompile: aString<br>
> + in: selectedClassOrMetaClass<br>
> + classified: category<br>
> + notifying: aController.<br>
> - selector := selectedClassOrMetaClass<br>
> - compile: aString<br>
> - classified: category<br>
> - notifying: aController.<br>
> selector == nil ifTrue: [^ nil].<br>
> contents := aString copy.<br>
> currentCompiledMethod := selectedClassOrMetaClass compiledMethodAt: selector.<br>
> ^ true!<br>
><br>
> Item was changed:<br>
> ----- Method: DependencyBrowser>>defineMessageFrom:notifying: (in category 'contents') -----<br>
> defineMessageFrom: aString notifying: aController<br>
> "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."<br>
> | selectedMessageName selector category oldMessageList |<br>
> selectedMessageName := self selectedMessageName.<br>
> oldMessageList := self messageList.<br>
> contents := nil.<br>
> selector := (self selectedClassOrMetaClass newParser parseSelector: aString).<br>
> + selector := self<br>
> + basicCompile: aString<br>
> + in: self selectedClassOrMetaClass<br>
> + classified: (category := self selectedMessageCategoryName)<br>
> + notifying: aController.<br>
> - selector := self selectedClassOrMetaClass<br>
> - compile: aString<br>
> - classified: (category := self selectedMessageCategoryName)<br>
> - notifying: aController.<br>
> selector == nil ifTrue: [^ false].<br>
> contents := aString copy.<br>
> ^ true<br>
> !<br>
><br>
> Item was changed:<br>
> AppRegistry subclass: #SystemBrowser<br>
> instanceVariableNames: ''<br>
> + classVariableNames: 'AcceptWithPrettyPrint BrowseWithDragNDrop BrowseWithPrettyPrint'<br>
> - classVariableNames: 'BrowseWithDragNDrop BrowseWithPrettyPrint'<br>
> poolDictionaries: ''<br>
> category: 'Tools-Base'!<br>
><br>
> !SystemBrowser commentStamp: '<historical>' prior: 0!<br>
> This is the AppRegistry class for class browsing!<br>
><br>
> Item was added:<br>
> + ----- Method: SystemBrowser class>>acceptWithPrettyPrint (in category 'preferences') -----<br>
> + acceptWithPrettyPrint<br>
> + <preference: 'Accept with pretty-print' category: 'browsing' description: 'If true, browsers will automatically pretty-print every method when you accept it.' type: #Boolean><br>
> + ^ AcceptWithPrettyPrint ifNil: [false].!<br>
><br>
> Item was added:<br>
> + ----- Method: SystemBrowser class>>acceptWithPrettyPrint: (in category 'preferences') -----<br>
> + acceptWithPrettyPrint: aBoolean<br>
> + AcceptWithPrettyPrint := aBoolean.!<br>
><br>
><br>
<br>
</blockquote></div>