On Thu, Aug 26, 2010 at 22:26:26 +0300, Dmitry Astapov wrote: > 1)"darcs add" without "--reserved-ok" will abnormally terminate with > "fromJust: Nothing" > 2)"darcs add" with "--reserved-ok" will terminate in exactly the same way
Great (and thanks for marking Kirstin's bug a duplicate). Nice to see we're starting to nail things down properly. > So, colon is a special case among all reserved characters, and it is > solely because the same function (isFile) is used to distinguish > between local and remote repositories in "darcs get" and "darcs put" > and to distinguish between absolute and relative filepaths in > isRelative. Right, and I have trouble keeping the two thoughts in my head at the same time (colon for Windows, and colon for SSH). You push one thought in, and the other thought slides out. > > > Unless we make a drastical change and start requiring "scp://" in > > > front of the scp repo names, all colons in repo dir names should be > > > disallowed. > > > > *Requiring* ssh repos to be URIs could be tricky and perhaps involve > > a painful deprecation process (which isn't to say we shouldn't do it > > just that it will take some dedication) > > > > It would definitely be nice to implement them as a feature. Then a smooth > plan for phasing-out old repo names could be devised. Sounds good. > According to the comments in URL.hs, path like "C:blah-blah-blah" is > considered absolute under Windows with rationale that mentioning drive name > in path makes it absolute. In fact, it is not the case, since "C:blah" is > relative to current working dir on drive C: and only stuff like "C:\blah" > would be absolute. > > So I treated that clause as a bug lying in waiting and removed it. It may even be worth filing a ticket for, not sure. At the very least mentioning in the patch comments > > > 1)Replace isRelative from URL.hs by isRelative from System.FilePath, > > which > > > should take into account differences between platforms and handle colon > > > under unix in sensible way. > > > > Would isFile && System.FilePath.isRelative have the same benefits? > > > > I don't really understand the question. Please note that all current call > sites for isFile are left intact, and code for isFile is left intact. Only > isRelative is changed, and it is used in the single place (RepoPath.hs), and > thus it is easy to reason about possible repercussions. See more below. I saw this particular bit of the hunk: -isRelative f@(c:_) = isFile f && c /= '/' && c /= '~' And I was afraid of losing the check, but on further inspection isFile has the undesired checking-for-colon behaviour (which I think you were saying and I was just missing). So sorry, I was just being silly. One possibility actually is if isRelative is only used by RepoPath.hs to just nuke it altogether and have RepoPath do the reasoning via FilePath. There we'll have to ask ourselves if at RepoPath time, we need to care about remote paths or not, which we probably don't > Since isRelative should really-really-really detect the relative paths, and > do it in platform-specific way, I believe that System.FilePath is the way to > go in this particular case. Sounds convincing. > Summarizing: I still insist that my patch fixes the issue at hand, and has > no ill side-effects (pun intended). OK! So I'm a lot less nervous about this having slept on it (I realised that this is actually still quite surface level stuff that shouldn't affect any of the inner patch-level stuff). Let me see if I can find you a patch reviewer. Meanwhile, could I abuse you to somehow make our path testing logic much more aggressive and evil? -- Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow> For a faster response, try +44 (0)1273 64 2905 or xmpp:[email protected] (Jabber or Google Talk only)
signature.asc
Description: Digital signature
_______________________________________________ darcs-users mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-users
