Re: [PATCH 3/7] Extract, test and enhance the logic to collapse ../foo paths.

2012-09-26 Thread Jonathan Nieder
Hi, Michael G Schwern wrote: On 2012.7.30 12:51 PM, Eric Wong wrote: Michael G Schwern wrote: _collapse_dotdot() works better than the existing regex did. I don't dispute it's better, but it's worth explaining in the commit message to reviewers why something is better. Yeah. I figured

Re: [PATCH 3/7] Extract, test and enhance the logic to collapse ../foo paths.

2012-09-26 Thread Eric Wong
Jonathan Nieder jrnie...@gmail.com wrote: Hi, Michael G Schwern wrote: On 2012.7.30 12:51 PM, Eric Wong wrote: Michael G Schwern wrote: _collapse_dotdot() works better than the existing regex did. I don't dispute it's better, but it's worth explaining in the commit message to

Re: [PATCH 3/7] Extract, test and enhance the logic to collapse ../foo paths.

2012-09-26 Thread Jonathan Nieder
Eric Wong wrote: That said, I'd favor an implementation that split on m{/+} and collapsed as Michael mentioned. Sounds sensible. Is canonicalize_path responsible for collapsing runs of slashes? What should _collapse_dotdot do to c:/.. or http://www.example.com/..;? -- To unsubscribe from

Re: [PATCH 3/7] Extract, test and enhance the logic to collapse ../foo paths.

2012-09-26 Thread Eric Wong
Jonathan Nieder jrnie...@gmail.com wrote: Eric Wong wrote: That said, I'd favor an implementation that split on m{/+} and collapsed as Michael mentioned. Sounds sensible. Is canonicalize_path responsible for collapsing runs of slashes? What should _collapse_dotdot do to c:/.. or

Re: [PATCH 3/7] Extract, test and enhance the logic to collapse ../foo paths.

2012-09-26 Thread Jonathan Nieder
Eric Wong wrote: It should probably just return the root path (c:/ and http://www.example.com/; respectively). That means recognizing drive letters and URLs. Hm. Subversion commands seem to use svn_client_args_to_target_array2 to canonicalize arguments. It does something like the following:

Re: [PATCH 3/7] Extract, test and enhance the logic to collapse ../foo paths.

2012-09-26 Thread Eric Wong
Jonathan Nieder jrnie...@gmail.com wrote: Maybe we can use apr_filepath_merge() to avoid reinventing the wheel? Ideally, yes. Is there an easy way to access that from Perl? (and for the older versions of SVN folks people are running). Perhaps we can expose equivalent functionality in git via

Re: [PATCH 3/7] Extract, test and enhance the logic to collapse ../foo paths.

2012-09-26 Thread Jonathan Nieder
Eric Wong wrote: Ideally, yes. Is there an easy way to access that from Perl? (and for the older versions of SVN folks people are running). Subversion's swig bindings only wrap a few apr functions and do not depend on fuller apr bindings. Something like svn_dirent_is_under_root() could be

Re: [PATCH 3/7] Extract, test and enhance the logic to collapse ../foo paths.

2012-07-30 Thread Eric Wong
Michael G. Schwern schw...@pobox.com wrote: From: Michael G. Schwern schw...@pobox.com The SVN API functions will not accept ../foo but their canonicalization functions will not collapse it. So we'll have to do it ourselves. _collapse_dotdot() works better than the existing regex did. I

Re: [PATCH 3/7] Extract, test and enhance the logic to collapse ../foo paths.

2012-07-30 Thread Michael G Schwern
On 2012.7.30 12:51 PM, Eric Wong wrote: The SVN API functions will not accept ../foo but their canonicalization functions will not collapse it. So we'll have to do it ourselves. _collapse_dotdot() works better than the existing regex did. I don't dispute it's better, but it's worth