Re: [Pharo-users] (MimeConverter forEncoding: 'quoted-printable') subclass responsibility
Hi Sven, Sven, sorry for my late reply. On the weekends, I try to be away from Smalltalk. Sometimes I succeed, sometimes I don't ;-) So first of all, thanks for porting the code and even adding those nice comments. I like the way "my" code looks now ;-) About the CRLF stuff: this is a complicated area, even if it looks so simple. As I mentioned, VAST is not yet Unicode aware, so it adapts itself to the current platform's line-end conversion. So what I did to build tests fast was just add line breaks in literals in the test code and let envy handle the rest. That is, of course, neither a nice coding convention, nor a perfect solution. I don't know much about Pharo's handling of line-end conventions of its host platform, so I cannot comment on the correct way to do this in Pharo. It is important in VAST to convert to the correct line-endings of the OS in VAST, and for this VAST provides some pool constants: CldtConstants::UNIXLineDelimiter and CldtConstants::WinLineDelimiter. As I understand it, you Pharo guys don't like Pool Dictionaries very deerly, so I guess there is another place to ask for this if needed. So in the end, I cannot really judge if your changes are correct. You'd have to test on both Win and Mac/Linux to see if your changes are correct. For me, String>>crlf sounds wrong, because it it always returns both cr and lf. If Pharo also uses the local line-end convention, this would be wrong in Unix/Mac. I hope this long version of "I have no idea" is helpful... ;-) Joachim Am 06.12.13 20:18, schrieb Sven Van Caekenberghe: Hi Joachim, I loaded/copied your code into Pharo 3.0 and made the necessary changes, as well as some simplifications, to make everything work. See the attached fileouts. The tests were almost immediately green. I have one question though, I had to change two tests to make them work: #testDecodeCRLF self assert: (coder decode: 'This is the first line=0D=0A and this is the second') equals: 'This is the first line', String crlf, ' and this is the second’ You originally had a simple line break in the method’s string literal. But that results in a CR in Pharo. I added an explicit CRLF. I had to make similar changes to #testEncodeSoftLinebreak. Is that OK or not ? What do you think ? Sven On 06 Dec 2013, at 12:52, Sven Van Caekenberghe wrote: Hi Joachim, The code is perfectly readable, including the German (which I can read/understand more or less, speaking/writing is quite bad though), thanks a lot. I will try to port it later today or this evening. Sven On 06 Dec 2013, at 12:26, jtuc...@objektfabrik.de wrote: Hi Sven, I just see there will be some problems with the test Strings in Pharo. VAST is not Unicode aware yet, so the fileout of german umlauts and such will very likely be wrong when pasting into Pharo. I am happy to send you corrections for the decoded strings in the test code if your German doesn't suffice. Just tell me which sentences are imported wrong into Pharo... I have never tested any other special characters than German ones. Joachim Am 06.12.13 12:14, schrieb jtuc...@objektfabrik.de: Hi Sven, here you are. It's possible you get into minor troubles parsing the files, but the code is short enough to even copy/paste it into Pharo I guess. Since it is a fileout from VAST, you may need to tweak the categories for monticello and such. Please let me know if there are problems with the code, if you add new tests or find bugs. I'd like to add stuff back to my code then. Good luck, Joachim Am 06.12.13 12:06, schrieb Sven Van Caekenberghe: On 06 Dec 2013, at 12:04, jtuc...@objektfabrik.de wrote: Hi Sven, Of course I don't mind. I'll be happy if you find it useful enough to use it in Pharo. Great! The .dat is VAST's proprietary export format for code. So you'd have to install VAST to get it. I am not a Pharo contributor yet, so I will see if I can send you a fileout of the code that should beasier to import into Pharo. I know this is stupid, but I never worried about the license of such mini-contributions. So if you want, it's MIT ;-) Yes, please send some .st files. Joachim Am 06.12.13 11:36, schrieb Sven Van Caekenberghe: Hi Joachim, On 06 Dec 2013, at 11:22, jtuc...@objektfabrik.de wrote: Hi Sabine, this is not of immediate help for you, but I once wrote a QuotedPrintableEncode for VA Smalltalk. It is in productive use and should handle both encoding and decoding just fine. You can find it on vastgoodies.com and I am sure it is not hard to port to Pharo. So I guess you are OK with me looking at / porting your code ? Is it MIT licensed or compatible ? Are you already a signed up Pharo contributor ? How do you I get at the actual code ? When doing a download, I get a .dat file. How do I open that ? Sven More details: http://joachimtuchel.wordpress.com/2011/09/23/quotedprintablecoder-on-vastgoodies/ HTH, Joachim Am 06.12.13 11:11, schrieb Sa
Re: [Pharo-users] (MimeConverter forEncoding: 'quoted-printable') subclass responsibility
Joachim, On 08 Dec 2013, at 10:06, jtuc...@objektfabrik.de wrote: > Hi Sven, > Sven, > > sorry for my late reply. On the weekends, I try to be away from Smalltalk. > Sometimes I succeed, sometimes I don't ;-) No problem! > So first of all, thanks for porting the code and even adding those nice > comments. I like the way "my" code looks now ;-) The code was quite readable and clean to begin with ! > About the CRLF stuff: this is a complicated area, even if it looks so simple. > As I mentioned, VAST is not yet Unicode aware, so it adapts itself to the > current platform's line-end conversion. So what I did to build tests fast was > just add line breaks in literals in the test code and let envy handle the > rest. That is, of course, neither a nice coding convention, nor a perfect > solution. I don't know much about Pharo's handling of line-end conventions of > its host platform, so I cannot comment on the correct way to do this in Pharo. > > It is important in VAST to convert to the correct line-endings of the OS in > VAST, and for this VAST provides some pool constants: > CldtConstants::UNIXLineDelimiter and CldtConstants::WinLineDelimiter. As I > understand it, you Pharo guys don't like Pool Dictionaries very deerly, so I > guess there is another place to ask for this if needed. > > So in the end, I cannot really judge if your changes are correct. You'd have > to test on both Win and Mac/Linux to see if your changes are correct. For me, > String>>crlf sounds wrong, because it it always returns both cr and lf. If > Pharo also uses the local line-end convention, this would be wrong in > Unix/Mac. > > I hope this long version of "I have no idea" is helpful... ;-) > > Joachim Inside a Pharo image, CR is the line end on all platforms (Windows, Mac, Linux). Going to the outside world (files, sockets, clipboard), there are options for conversion or automatic conversion, or none at all. When reading lines, most code can handle all 3 conventions transparently. I modified the code to assume CR as Pharo platform standard line end convention when decoding, when encoding any line end convention works. I intend to make the actual line end convention a parameter, defaulting to CR. I added an error when the char code is > 255. I added/changed a couple of tests. Please have a look. Next I will actually move your code to the instance side of QuotedPrintableMimeConverter, the class where we actually need it. I am sure I can take over your units tests as they are, which is good. Sven QuotedPrintableCoder.st Description: Binary data QuotedPrintableCoderTest.st Description: Binary data > Am 06.12.13 20:18, schrieb Sven Van Caekenberghe: >> Hi Joachim, >> >> I loaded/copied your code into Pharo 3.0 and made the necessary changes, as >> well as some simplifications, to make everything work. See the attached >> fileouts. >> >> The tests were almost immediately green. >> >> I have one question though, I had to change two tests to make them work: >> >> #testDecodeCRLF >> >> self >>assert: (coder decode: 'This is the first line=0D=0A and this is the >> second') >>equals: 'This is the first line', String crlf, ' and this is the second’ >> >> You originally had a simple line break in the method’s string literal. But >> that results in a CR in Pharo. I added an explicit CRLF. >> >> I had to make similar changes to #testEncodeSoftLinebreak. >> >> Is that OK or not ? What do you think ? >> >> Sven >> >> >> >> >> >> >> On 06 Dec 2013, at 12:52, Sven Van Caekenberghe wrote: >> >>> Hi Joachim, >>> >>> The code is perfectly readable, including the German (which I can >>> read/understand more or less, speaking/writing is quite bad though), thanks >>> a lot. I will try to port it later today or this evening. >>> >>> Sven >>> >>> On 06 Dec 2013, at 12:26, jtuc...@objektfabrik.de wrote: >>> Hi Sven, I just see there will be some problems with the test Strings in Pharo. VAST is not Unicode aware yet, so the fileout of german umlauts and such will very likely be wrong when pasting into Pharo. I am happy to send you corrections for the decoded strings in the test code if your German doesn't suffice. Just tell me which sentences are imported wrong into Pharo... I have never tested any other special characters than German ones. Joachim Am 06.12.13 12:14, schrieb jtuc...@objektfabrik.de: > Hi Sven, > > here you are. > It's possible you get into minor troubles parsing the files, but the code > is short enough to even copy/paste it into Pharo I guess. Since it is a > fileout from VAST, you may need to tweak the categories for monticello > and such. > > Please let me know if there are problems with the code, if you add new > tests or find bugs. I'd like to add stuff back to my code then. > > Good luck, > > Joachim >
Re: [Pharo-users] (MimeConverter forEncoding: 'quoted-printable') subclass responsibility
On 08 Dec 2013, at 19:51, Sven Van Caekenberghe wrote: > Next I will actually move your code to the instance side of > QuotedPrintableMimeConverter, the class where we actually need it. I am sure > I can take over your units tests as they are, which is good. Done: https://pharo.fogbugz.com/f/cases/4825/Quoted-printable-encoding-incomplete RESOLVED Slice in 3.0 inbox: === Name: SLICE-Issue-4825-Quoted-printable-encoding-incomplete-SvenVanCaekenberghe.1 Author: SvenVanCaekenberghe Time: 8 December 2013, 11:45:09.148454 pm UUID: af015099-e48b-47f5-9553-840fca62d8e2 Ancestors: Dependencies: Network-MIME-SvenVanCaekenberghe.64, NetworkTests-SvenVanCaekenberghe.92 A completely new implementation of QuotedPrintableMimeConverter based on code contributed by Joachim Tuchel (thanks !) including a new set of unit tests. Note that the older code lacked the encoding part and had no tests at all. === Sven