<div id="__MailbirdStyleContent" style="font-size: 10pt;font-family: Arial;color: #000000;text-align: left" dir="ltr">
Hi Christoph --<div><br></div><div><span style="font-family: Arial, Helvetica, sans-serif;font-size: 13px">> So can I go ahead and merge this? :-)</span><br></div><div><span style="font-family: Arial, Helvetica, sans-serif;font-size: 13px"><br></span></div><div><span style="font-family: Arial, Helvetica, sans-serif;font-size: 13px">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?</span></div><div><span style="font-family: Arial, Helvetica, sans-serif;font-size: 13px"><br></span></div><div><span style="font-family: Arial, Helvetica, sans-serif;font-size: 13px">Maybe something listening (addDependent/change/update) to "aPackageInfo preamble"? A StringHolder has a custom collection of dependents...</span></div><div><span style="font-family: Arial, Helvetica, sans-serif;font-size: 13px"><br></span></div><div><span style="font-family: Arial, Helvetica, sans-serif;font-size: 13px">Best,</span></div><div><span style="font-family: Arial, Helvetica, sans-serif;font-size: 13px">Marcel</span></div><div class="mb_sig"></div>
<blockquote class="history_container" type="cite" style="border-left-style: solid;border-width: 1px;margin-top: 20px;margin-left: 0px;padding-left: 10px;min-width: 500px">
<p style="color: #AAAAAA; margin-top: 10px;">Am 22.11.2021 16:38:31 schrieb christoph.thiede@student.hpi.uni-potsdam.de <christoph.thiede@student.hpi.uni-potsdam.de>:</p><div style="font-family:Arial,Helvetica,sans-serif">
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>
<b>=============== Summary ===============</b><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>
<b>=============== Postscript ===============</b><br>
<br>
<span style="color: #008080">"Postscript:<br>
Eliminate StringHolder script in PackageInfos"</span><span style="color: #000000"><br>
PackageInfo </span><span style="color: #000080">allSubInstancesDo:</span><span style="color: #000000"> [:</span><span style="color: #000080">packageInfo</span><span style="color: #000000"> </span><span style="color: #808080">|</span><span style="color: #000000"><br>
</span><span style="color: #000080">#</span><span style="color: #000000">(</span><span style="color: #000080">preamble</span><span style="color: #000000"> </span><span style="color: #000080">postscript</span><span style="color: #000000"> </span><span style="color: #000080">preambleOfRemoval</span><span style="color: #000000"> </span><span style="color: #000080">postscriptOfRemoval</span><span style="color: #000000">) </span><span style="color: #000080">do:</span><span style="color: #000000"> </span><span style="color: #008000">[</span><span style="color: #000000">:</span><span style="color: #000080">sel</span><span style="color: #000000"> </span><span style="color: #808080">|</span><span style="color: #000000"><br>
</span><span style="color: #800080">(</span><span style="color: #000080">packageInfo</span><span style="color: #000000"> </span><span style="color: #000080">instVarNamed:</span><span style="color: #000000"> </span><span style="color: #000080">sel</span><span style="color: #800080">)</span><span style="color: #000000"> </span><span style="color: #000080">ifNotNil:</span><span style="color: #000000"> </span><span style="color: #800080">[</span><span style="color: #000000">:</span><span style="color: #000080">stringHolder</span><span style="color: #000000"> </span><span style="color: #808080">|</span><span style="color: #000000"><br>
</span><span style="color: #000080">packageInfo</span><span style="color: #000000"> </span><span style="color: #000080">instVarNamed:</span><span style="color: #000000"> </span><span style="color: #000080">sel</span><span style="color: #000000"> </span><span style="color: #000080">put:</span><span style="color: #000000"> </span><span style="color: #000080">stringHolder</span><span style="color: #000000"> </span><span style="color: #000080">contents</span><span style="color: #800080">]</span><span style="color: #008000">]</span><span style="color: #000000">].</span><br>
<br>
<b>=============== Diff ===============</b><br>
<br>
<b>MCStWriter>>writeScriptDefinition: {writing} · ct 11/22/2021 16:34 (changed)</b><br>
writeScriptDefinition: definition<br>
stream nextChunkPut: (<br>
'(PackageInfo named: {1}) {2}: {3}'<br>
format: {<br>
"{1}" definition packageName printString.<br>
"{2}" definition scriptSelector. <br>
<s><span style="color: #0000FF">- "{3}" definition script printString<br>
</span></s><span style="color: #FF0000">+ "{3}" definition script asString printString<br>
</span> }); cr<br>
<br>
<b>MCWorkingCopyBrowser>>editLoadScripts {morphic ui} · ct 11/22/2021 16:05 (changed)</b><br>
editLoadScripts<br>
<br>
| arg |<br>
self hasWorkingCopy ifFalse: [^self].<br>
arg := UIManager default<br>
<s><span style="color: #0000FF">- chooseFrom: #('edit preamble' 'edit postscript' 'edit preambleOfRemoval' 'edit postscriptOfRemoval')<br>
</span></s><span style="color: #FF0000">+ chooseFrom: {'edit preamble' translated. 'edit postscript' translated. 'edit preambleOfRemoval' translated. 'edit postscriptOfRemoval' translated}<br>
</span> values: #(#preamble #postscript #preambleOfRemoval #postscriptOfRemoval).<br>
<br>
arg ifNotNil: [<br>
self editScript: arg].<br>
<br>
<b>MCWorkingCopyBrowser>>editScript: {morphic ui} · ct 11/22/2021 15:59 (changed)</b><br>
editScript: scriptSymbol<br>
<br>
<s><span style="color: #0000FF">- | script |<br>
- script := workingCopy packageInfo perform: scriptSymbol.<br>
- script openLabel: scriptSymbol asString, ' of the Package ', workingCopy package name.<br>
</span></s><span style="color: #FF0000">+ | 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]</span><br>
<br>
<b>PackageInfo>>isScript:not: {preamble/postscript} · ct 11/22/2021 15:53 (changed)</b><br>
isScript: script not: default<br>
^ script notNil<br>
and: [ | contents |<br>
<s><span style="color: #0000FF">- contents := script contents asString withBlanksTrimmed.<br>
</span></s><span style="color: #FF0000">+ contents := script asString withBlanksTrimmed.<br>
</span> contents notEmpty and: [contents ~= default and: [contents ~= 'nil']]]<br>
<br>
<b>PackageInfo>>postscript {preamble/postscript} · ct 11/22/2021 15:53 (changed)</b><br>
postscript<br>
<span style="color: #FF0000">+ <br>
</span> ^ postscript ifNil: [<br>
<s><span style="color: #0000FF">- postscript := StringHolder new contents: self postscriptDefault]<br>
</span></s><span style="color: #FF0000">+ postscript := self postscriptDefault]</span><br>
<br>
<b>PackageInfo>>postscript: {preamble/postscript} · ct 11/22/2021 15:57 (changed)</b><br>
postscript: aString<br>
<br>
<s><span style="color: #0000FF">- postscript := StringHolder new contents: aString<br>
</span></s><span style="color: #FF0000">+ postscript := aString</span><br>
<br>
<b>PackageInfo>>postscriptOfRemoval {preamble/postscript} · ct 11/22/2021 15:53 (changed)</b><br>
postscriptOfRemoval<br>
^ postscriptOfRemoval ifNil: [<br>
<s><span style="color: #0000FF">- postscriptOfRemoval := StringHolder new contents: self postscriptOfRemovalDefault]<br>
</span></s><span style="color: #FF0000">+ postscriptOfRemoval := self postscriptOfRemovalDefault]</span><br>
<br>
<b>PackageInfo>>postscriptOfRemoval: {preamble/postscript} · ct 11/22/2021 15:57 (changed)</b><br>
postscriptOfRemoval: aString<br>
<br>
<s><span style="color: #0000FF">- postscriptOfRemoval := StringHolder new contents: aString<br>
</span></s><span style="color: #FF0000">+ postscriptOfRemoval := aString<br>
</span><br>
<br>
<b>PackageInfo>>preamble {preamble/postscript} · ct 11/22/2021 15:54 (changed)</b><br>
preamble<br>
^ preamble ifNil: [<br>
<s><span style="color: #0000FF">- preamble := StringHolder new contents: self preambleDefault]<br>
</span></s><span style="color: #FF0000">+ preamble := self preambleDefault]</span><br>
<br>
<b>PackageInfo>>preamble: {preamble/postscript} · ct 11/22/2021 15:56 (changed)</b><br>
preamble: aString<br>
<br>
<s><span style="color: #0000FF">- preamble := StringHolder new contents: aString<br>
</span></s><span style="color: #FF0000">+ preamble := aString</span><br>
<br>
<b>PackageInfo>>preambleOfRemoval {preamble/postscript} · ct 11/22/2021 15:54 (changed)</b><br>
preambleOfRemoval<br>
^ preambleOfRemoval ifNil: [<br>
<s><span style="color: #0000FF">- preambleOfRemoval := StringHolder new contents: self preambleOfRemovalDefault]<br>
</span></s><span style="color: #FF0000">+ preambleOfRemoval := self preambleOfRemovalDefault]</span><br>
<br>
<b>PackageInfo>>preambleOfRemoval: {preamble/postscript} · ct 11/22/2021 15:56 (changed)</b><br>
preambleOfRemoval: aString<br>
<br>
<s><span style="color: #0000FF">- preambleOfRemoval := StringHolder new contents: aString<br>
</span></s><span style="color: #FF0000">+ preambleOfRemoval := aString</span><br>
<br>
<span style="color: #808080">---<br>
</span><span style="color: #808080"><i>Sent from </i></span><span style="color: #808080"><i><a href="https://github.com/hpi-swa-lab/squeak-inbox-talk"><u><span style="color: #808080">Squeak Inbox Talk</span></u></a></i></span><br>
<br>
On 2021-11-22T16:22:02+01:00, marcel.taeumel@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>
>
</div></blockquote></div>