> Overall, I agree that url handling in the various transports classes may
> receive some love especially about which encoding is used (including url-
> escaped) is/should/should not be used by which methods.
Yes, Jelmer has gone some way towards sorting this out, but there's still a lot of confusion in the code.
> It's both amazing and scary that you've been able to fix this bug without
> touching a single transport test... I understand why but it's still amazing.
Well, that's due to the cheat with split_trailing_slash which lets me replicate the existing (somewhat bogus) behaviour but without tickling the main problem by trying to split/join as a url.
> Hmm, do I see a pattern there ?
>
> This screams: split these tests to my ears.
Yes, and given it should really cover the case with schemes as well, it almost wants parametrizing. I resisted that as part of this change as I'm unconvinced by the current interfaces.
> Two final notes:
> - there has been discussion in the past about never removing the final path
> from 't.base' as it *is* a directory anyway (and then stop adding it blindly),
> - I'd really like to see your mail summarising your thoughts about url
> handling across the code base ;-D
Being far more careful about what are valid operations on different kinds of url scheme would help, which perhaps the URL class can assist with though I'm a little leery of the added weight there.
> Overall, I agree that url handling in the various transports classes may
> receive some love especially about which encoding is used (including url-
> escaped) is/should/should not be used by which methods.
Yes, Jelmer has gone some way towards sorting this out, but there's still a lot of confusion in the code.
> It's both amazing and scary that you've been able to fix this bug without
> touching a single transport test... I understand why but it's still amazing.
Well, that's due to the cheat with split_trailing_ slash which lets me replicate the existing (somewhat bogus) behaviour but without tickling the main problem by trying to split/join as a url.
> Hmm, do I see a pattern there ?
>
> This screams: split these tests to my ears.
Yes, and given it should really cover the case with schemes as well, it almost wants parametrizing. I resisted that as part of this change as I'm unconvinced by the current interfaces.
> Two final notes:
> - there has been discussion in the past about never removing the final path
> from 't.base' as it *is* a directory anyway (and then stop adding it blindly),
> - I'd really like to see your mail summarising your thoughts about url
> handling across the code base ;-D
Being far more careful about what are valid operations on different kinds of url scheme would help, which perhaps the URL class can assist with though I'm a little leery of the added weight there.
> .. and one more thing: news entry
Added, and nits addressed.