On Thu, May 27, 2010 at 12:15:56 +0000, Petr Ročkai wrote: > the patch itself should be self-explanatory. The catch was, that conflictors > were written out using showPatch which uses the legacy OldFormat encoding. In > reading them, this is not reflected and the result is bogus conflictors.
Nice work. I hesitate to apply this because I'm not paying enough this patch the attention it deserves, but in case my comments help Petr or another reviewer... The context behind this patch is that we have two styles of filename, the second style being introduced in Darcs 2 (Petr claims that that the new format patches are only used for darcs-2 patches and not say darcs-1 patches. I hope that's right, but in any case it's unlikely that darcs-2 patches would use OldFormat!). (Code is slightly paraphrased for hopefully clarity) data FileNameFormat = OldFormat | NewFormat readFileName :: FileNameFormat -> B.ByteString -> FileName readFileName OldFormat = fp2fn . decodeWhite . unpackPSFromUTF8 readFileName NewFormat = fp2fn . decodeWhite . BC.unpack formatFileName :: FileNameFormat -> FileName -> Doc formatFileName OldFormat = packedString . packStringToUTF8 . encodeWhite . fn2fp formatFileName NewFormat = text . encodeWhite . fn2fp (packedString and text are just wrappers around the Doc constructors) So in the OldFormat we seem to assume that Darcs.Patch.FileName uses Unicode filenames encoded in UTF-8. Does this mean that in the NewFormat, we just treat filenames as just sequences of bytes? If so that (very superficially and unthinkingly said) sounds like a step backwards. I wonder what exactly the thinking behind this was... One thing that worries me though is if printing filenames will ever locale-dependent in GHC 6.12. I seem to remember that the put/get audit said that we're always outputting in binary mode where it matters (and so treating the Strings as padded octets a la GHC 6.10) > Ideally, we'd just ban showPatch use on Prims, but this would require further > changes. For now, we should hope that this does not happen anywhere else in > darcs-2 patch core. Alternatively, we would have patches that know if they are NewFormat or OldFormat (and then safely use showPatch?) > +showPrimFL :: FileNameFormat -> FL Prim C(a b) -> Doc > +showPrimFL f xs = vcat (mapFL (showPrim f) xs) Presumably this comes from the instance of ShowPatch for FL (which makes sense). > - showPatch cs $$ > + showPrimFL NewFormat cs $$ What about showNon and the rotcilfnoc case? Are there other cases where we'd need to pay attention to this? -- Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow> PGP Key ID: 08AC04F9
signature.asc
Description: Digital signature
_______________________________________________ darcs-users mailing list darcs-users@darcs.net http://lists.osuosl.org/mailman/listinfo/darcs-users
