Hi Marcel,<br>
<br>
> Could you figure out a reason why this has been a StringHolder until now?<br>
<br>
Unfortunately, I have no idea. I saw the same pattern for the do-it list (see Utilities class>>#editCommonRequestStrings), maybe it was just a cheap hack in a time where workspaces did not yet have an accept selector?<br>
<br>
> Maybe something listening (addDependent/change/update) to "aPackageInfo preamble"? A StringHolder has a custom collection of dependents...<br>
<br>
When I edit a package script (or also edit my do-it list), the dependents list only consists of Morphic views ...<br>
<br>
Best,<br>
Christoph<br>
<br>
<font color="#808080">---<br>
</font><font color="#808080"><i>Sent from </i></font><font color="#808080"><i><a href="https://github.com/hpi-swa-lab/squeak-inbox-talk"><u><font color="#808080">Squeak Inbox Talk</font></u></a></i></font><br>
<br>
On 2021-11-22T17:42:31+01:00, marcel.taeumel@hpi.de wrote:<br>
<br>
> Hi Christoph --<br>
> <br>
> > So can I go ahead and merge this? :-)<br>
> <br>
> <br>
> Could you figure out a reason why this has been a StringHolder until now? Was it because String itself is considered "immutable" and StringHolder was its "mutable" container?<br>
> <br>
> Maybe something listening (addDependent/change/update) to "aPackageInfo preamble"? A StringHolder has a custom collection of dependents...<br>
> <br>
> Best,<br>
> Marcel<br>
> Am 22.11.2021 16:38:31 schrieb christoph.thiede at student.hpi.uni-potsdam.de <christoph.thiede at student.hpi.uni-potsdam.de>:<br>
> Hi Marcel,<br>
> <br>
> thanks for the fast feedback!<br>
> <br>
> > Please double-check to not upload texts (or their serialized form) to the server.<br>
> <br>
> Good catch! ["stylePackageScripts.3.cs"] resolves this by converting the script to a string prior to serialization (see MCStWriter >> #writeScriptDefinition:). Updated diff is below. Addressing this issue in the writer seems to be the safest way to me, and you will not loose your formatting prior to exporting the package, which aligns to the behavior of class comments and method comments.<br>
> <br>
> (However, I would actually love to finally be able to serialize formatted comments via Monticello - but that should be stuff for its own debate.)<br>
> <br>
> So can I go ahead and merge this? :-)<br>
> <br>
> Best,<br>
> Christoph<br>
> <br>
> <br>
> =============== Summary ===============<br>
> <br>
> Change Set: stylePackageScripts<br>
> Date: 22 November 2021<br>
> Author: Christoph Thiede<br>
> <br>
> This change set enables syntax highlighting for package scripts ({preamble,postscript}{, of removal}). To achieve this, the instance variables in PackageInfo are converted to hold plain strings or texts rather than StringHolders, and the client (Monticello) is reponsible itself for displaying them to the user and storing them back into the package. Note the postscript which converts all PackageInfo instances in the image.<br>
> <br>
> =============== Postscript ===============<br>
> <br>
> "Postscript:<br>
> Eliminate StringHolder script in PackageInfos"<br>
> PackageInfo allSubInstancesDo: [:packageInfo |<br>
> #(preamble postscript preambleOfRemoval postscriptOfRemoval) do: [:sel |<br>
> (packageInfo instVarNamed: sel) ifNotNil: [:stringHolder |<br>
> packageInfo instVarNamed: sel put: stringHolder contents]]].<br>
> <br>
> =============== Diff ===============<br>
> <br>
> MCStWriter>>writeScriptDefinition: {writing} · ct 11/22/2021 16:34 (changed)<br>
> writeScriptDefinition: definition<br>
> stream nextChunkPut: (<br>
> '(PackageInfo named: {1}) {2}: {3}'<br>
> format: {<br>
> "{1}" definition packageName printString.<br>
> "{2}" definition scriptSelector.<br>
> - "{3}" definition script printString<br>
> + "{3}" definition script asString printString<br>
> }); cr<br>
> <br>
> MCWorkingCopyBrowser>>editLoadScripts {morphic ui} · ct 11/22/2021 16:05 (changed)<br>
> editLoadScripts<br>
> <br>
> | arg |<br>
> self hasWorkingCopy ifFalse: [^self].<br>
> arg := UIManager default<br>
> - chooseFrom: #('edit preamble' 'edit postscript' 'edit preambleOfRemoval' 'edit postscriptOfRemoval')<br>
> + chooseFrom: {'edit preamble' translated. 'edit postscript' translated. 'edit preambleOfRemoval' translated. 'edit postscriptOfRemoval' translated}<br>
> values: #(#preamble #postscript #preambleOfRemoval #postscriptOfRemoval).<br>
> <br>
> arg ifNotNil: [<br>
> self editScript: arg].<br>
> <br>
> MCWorkingCopyBrowser>>editScript: {morphic ui} · ct 11/22/2021 15:59 (changed)<br>
> editScript: scriptSymbol<br>
> <br>
> - | script |<br>
> - script := workingCopy packageInfo perform: scriptSymbol.<br>
> - script openLabel: scriptSymbol asString, ' of the Package ', workingCopy package name.<br>
> + | script |<br>
> + script := workingCopy packageInfo perform: scriptSymbol.<br>
> + Project uiManager<br>
> + edit: script<br>
> + label: ('{1} of the Package {2}' translated format: {scriptSymbol asString capitalized. workingCopy package name})<br>
> + shouldStyle: true<br>
> + accept: [:result | workingCopy packageInfo<br>
> + perform: scriptSymbol asSimpleSetter<br>
> + with: result]<br>
> <br>
> PackageInfo>>isScript:not: {preamble/postscript} · ct 11/22/2021 15:53 (changed)<br>
> isScript: script not: default<br>
> ^ script notNil<br>
> and: [ | contents |<br>
> - contents := script contents asString withBlanksTrimmed.<br>
> + contents := script asString withBlanksTrimmed.<br>
> contents notEmpty and: [contents ~= default and: [contents ~= 'nil']]]<br>
> <br>
> PackageInfo>>postscript {preamble/postscript} · ct 11/22/2021 15:53 (changed)<br>
> postscript<br>
> +<br>
> ^ postscript ifNil: [<br>
> - postscript := StringHolder new contents: self postscriptDefault]<br>
> + postscript := self postscriptDefault]<br>
> <br>
> PackageInfo>>postscript: {preamble/postscript} · ct 11/22/2021 15:57 (changed)<br>
> postscript: aString<br>
> <br>
> - postscript := StringHolder new contents: aString<br>
> + postscript := aString<br>
> <br>
> PackageInfo>>postscriptOfRemoval {preamble/postscript} · ct 11/22/2021 15:53 (changed)<br>
> postscriptOfRemoval<br>
> ^ postscriptOfRemoval ifNil: [<br>
> - postscriptOfRemoval := StringHolder new contents: self postscriptOfRemovalDefault]<br>
> + postscriptOfRemoval := self postscriptOfRemovalDefault]<br>
> <br>
> PackageInfo>>postscriptOfRemoval: {preamble/postscript} · ct 11/22/2021 15:57 (changed)<br>
> postscriptOfRemoval: aString<br>
> <br>
> - postscriptOfRemoval := StringHolder new contents: aString<br>
> + postscriptOfRemoval := aString<br>
> <br>
> <br>
> PackageInfo>>preamble {preamble/postscript} · ct 11/22/2021 15:54 (changed)<br>
> preamble<br>
> ^ preamble ifNil: [<br>
> - preamble := StringHolder new contents: self preambleDefault]<br>
> + preamble := self preambleDefault]<br>
> <br>
> PackageInfo>>preamble: {preamble/postscript} · ct 11/22/2021 15:56 (changed)<br>
> preamble: aString<br>
> <br>
> - preamble := StringHolder new contents: aString<br>
> + preamble := aString<br>
> <br>
> PackageInfo>>preambleOfRemoval {preamble/postscript} · ct 11/22/2021 15:54 (changed)<br>
> preambleOfRemoval<br>
> ^ preambleOfRemoval ifNil: [<br>
> - preambleOfRemoval := StringHolder new contents: self preambleOfRemovalDefault]<br>
> + preambleOfRemoval := self preambleOfRemovalDefault]<br>
> <br>
> PackageInfo>>preambleOfRemoval: {preamble/postscript} · ct 11/22/2021 15:56 (changed)<br>
> preambleOfRemoval: aString<br>
> <br>
> - preambleOfRemoval := StringHolder new contents: aString<br>
> + preambleOfRemoval := aString<br>
> <br>
> ---<br>
> Sent from Squeak Inbox Talk [https://github.com/hpi-swa-lab/squeak-inbox-talk]<br>
> <br>
> On 2021-11-22T16:22:02+01:00, marcel.taeumel at hpi.de wrote:<br>
> <br>
> > So, preamble and postscript would now be regular Strings instead? Sounds good to me, given that they are stored as strings on the server anyway.<br>
> ><br>
> > Please double-check to not upload texts (or their serialized form) to the server.<br>
> ><br>
> > Best,<br>
> > Marcel<br>
> > Am 22.11.2021 16:17:43 schrieb christoph.thiede at student.hpi.uni-potsdam.de <christoph.thiede at student.hpi.uni-potsdam.de>:<br>
> > Please give me a short feedback on whether you agree with the idea of this change (PackageInfo scripts are no longer instances of StringHolder). If there is no objection, I will try to merge this into the Trunk after a week or so. :-)<br>
> ><br>
> > ["stylePackageScripts.png"]<br>
> ><br>
> > Best,<br>
> > Christoph<br>
> ><br>
> ><br>
> > =============== Summary ===============<br>
> ><br>
> > Change Set: stylePackageScripts<br>
> > Date: 22 November 2021<br>
> > Author: Christoph Thiede<br>
> ><br>
> > This change set enables syntax highlighting for package scripts ({preamble,postscript}{, of removal}). To achieve this, the instance variables in PackageInfo are converted to hold plain strings or texts rather than StringHolders, and the client (Monticello) is reponsible itself for displaying them to the user and storing them back into the package. Note the postscript which converts all PackageInfo instances in the image.<br>
> ><br>
> > =============== Postscript ===============<br>
> ><br>
> > "Postscript:<br>
> > Eliminate StringHolder script in PackageInfos"<br>
> > PackageInfo allSubInstancesDo: [:packageInfo |<br>
> > #(preamble postscript preambleOfRemoval postscriptOfRemoval) do: [:sel |<br>
> > (packageInfo instVarNamed: sel) ifNotNil: [:stringHolder |<br>
> > packageInfo instVarNamed: sel put: stringHolder contents]]].<br>
> ><br>
> > =============== Diff ===============<br>
> ><br>
> > MCWorkingCopyBrowser>>editLoadScripts {morphic ui} · ct 11/22/2021 16:05 (changed)<br>
> > editLoadScripts<br>
> ><br>
> > | arg |<br>
> > self hasWorkingCopy ifFalse: [^self].<br>
> > arg := UIManager default<br>
> > - chooseFrom: #('edit preamble' 'edit postscript' 'edit preambleOfRemoval' 'edit postscriptOfRemoval')<br>
> > + chooseFrom: {'edit preamble' translated. 'edit postscript' translated. 'edit preambleOfRemoval' translated. 'edit postscriptOfRemoval' translated}<br>
> > values: #(#preamble #postscript #preambleOfRemoval #postscriptOfRemoval).<br>
> ><br>
> > arg ifNotNil: [<br>
> > self editScript: arg].<br>
> ><br>
> > MCWorkingCopyBrowser>>editScript: {morphic ui} · ct 11/22/2021 15:59 (changed)<br>
> > editScript: scriptSymbol<br>
> ><br>
> > - | script |<br>
> > - script := workingCopy packageInfo perform: scriptSymbol.<br>
> > - script openLabel: scriptSymbol asString, ' of the Package ', workingCopy package name.<br>
> > + | script |<br>
> > + script := workingCopy packageInfo perform: scriptSymbol.<br>
> > + Project uiManager<br>
> > + edit: script<br>
> > + label: ('{1} of the Package {2}' translated format: {scriptSymbol asString capitalized. workingCopy package name})<br>
> > + shouldStyle: true<br>
> > + accept: [:result | workingCopy packageInfo<br>
> > + perform: scriptSymbol asSimpleSetter<br>
> > + with: result]<br>
> ><br>
> > PackageInfo>>isScript:not: {preamble/postscript} · ct 11/22/2021 15:53 (changed)<br>
> > isScript: script not: default<br>
> > ^ script notNil<br>
> > and: [ | contents |<br>
> > - contents := script contents asString withBlanksTrimmed.<br>
> > + contents := script asString withBlanksTrimmed.<br>
> > contents notEmpty and: [contents ~= default and: [contents ~= 'nil']]]<br>
> ><br>
> > PackageInfo>>postscript {preamble/postscript} · ct 11/22/2021 15:53 (changed)<br>
> > postscript<br>
> > +<br>
> > ^ postscript ifNil: [<br>
> > - postscript := StringHolder new contents: self postscriptDefault]<br>
> > + postscript := self postscriptDefault]<br>
> ><br>
> > PackageInfo>>postscript: {preamble/postscript} · ct 11/22/2021 15:57 (changed)<br>
> > postscript: aString<br>
> ><br>
> > - postscript := StringHolder new contents: aString<br>
> > + postscript := aString<br>
> ><br>
> > PackageInfo>>postscriptOfRemoval {preamble/postscript} · ct 11/22/2021 15:53 (changed)<br>
> > postscriptOfRemoval<br>
> > ^ postscriptOfRemoval ifNil: [<br>
> > - postscriptOfRemoval := StringHolder new contents: self postscriptOfRemovalDefault]<br>
> > + postscriptOfRemoval := self postscriptOfRemovalDefault]<br>
> ><br>
> > PackageInfo>>postscriptOfRemoval: {preamble/postscript} · ct 11/22/2021 15:57 (changed)<br>
> > postscriptOfRemoval: aString<br>
> ><br>
> > - postscriptOfRemoval := StringHolder new contents: aString<br>
> > + postscriptOfRemoval := aString<br>
> ><br>
> ><br>
> > PackageInfo>>preamble {preamble/postscript} · ct 11/22/2021 15:54 (changed)<br>
> > preamble<br>
> > ^ preamble ifNil: [<br>
> > - preamble := StringHolder new contents: self preambleDefault]<br>
> > + preamble := self preambleDefault]<br>
> ><br>
> > PackageInfo>>preamble: {preamble/postscript} · ct 11/22/2021 15:56 (changed)<br>
> > preamble: aString<br>
> ><br>
> > - preamble := StringHolder new contents: aString<br>
> > + preamble := aString<br>
> ><br>
> > PackageInfo>>preambleOfRemoval {preamble/postscript} · ct 11/22/2021 15:54 (changed)<br>
> > preambleOfRemoval<br>
> > ^ preambleOfRemoval ifNil: [<br>
> > - preambleOfRemoval := StringHolder new contents: self preambleOfRemovalDefault]<br>
> > + preambleOfRemoval := self preambleOfRemovalDefault]<br>
> ><br>
> > PackageInfo>>preambleOfRemoval: {preamble/postscript} · ct 11/22/2021 15:56 (changed)<br>
> > preambleOfRemoval: aString<br>
> ><br>
> > - preambleOfRemoval := StringHolder new contents: aString<br>
> > + preambleOfRemoval := aString<br>
> > ["stylePackageScripts.2.cs"]<br>
> > -------------- next part --------------<br>
> > An HTML attachment was scrubbed...<br>
> > URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20211122/f05bc9a3/attachment.html><br>
> ><br>
> ><br>
> -------------- next part --------------<br>
> An HTML attachment was scrubbed...<br>
> URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20211122/a833bb10/attachment.html><br>
> <br>
>