dulwich-users team mailing list archive

[Dave Borowitz <dborowitz@xxxxxxxxxx>]

On Wed, Dec 22, 2010 at 00:34, Tay Ray Chuan <rctay89@xxxxxxxxx> wrote:

> Such replacements don't make sense for in-memory repos, only
> physical/disk-based ones.
>
> While doing so, refactor out some code into _url_to_path, renaming it
> too.
>
> This fixes test_get_idx_file and test_get_pack_file in
> test_web.DumbHandlersTestCase on Windows.
>

I would much prefer if instead you changed get_named_file to do the
os.path.sep substitution, and change its contract accordingly. That's
essentially an implementation detail of the BaseRepo subclass, so having to
deal with it at every call site where you want to map a logical -> physical
path seems wrong.


> ---
>
> I realised the content-types for idx and pack can also fit in 80 cols on
> a single line - made it so.
>
>  dulwich/web.py |   24 +++++++++++++-----------
>  1 files changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/dulwich/web.py b/dulwich/web.py
> index 3f354db..d8e8ee9 100644
> --- a/dulwich/web.py
> +++ b/dulwich/web.py
> @@ -112,16 +112,20 @@ def send_file(req, f, content_type):
>         raise
>
>
> -def _url_to_path(url):
> -    return url.replace('/', os.path.sep)
> +def _contents_and_path_for_url(backend, mat):
> +    repo = get_repo(backend, mat)
> +    path = mat.group()
> +    # os-specific separator for paths only make sense for physical repos
> +    if isinstance(repo, Repo):
>

I'm not a big fan of this type check. What if there's an alternate
disk-based repo implementation that doesn't inherit from repo.Repo?


> +        path = path.replace('/', os.path.sep)
> +    return repo.get_named_file(path), path
>
>
>  def get_text_file(req, backend, mat):
>     req.nocache()
> -    path = _url_to_path(mat.group())
> +    contents, path = _contents_and_path_for_url(backend, mat)
>     logger.info('Sending plain text file %s', path)
> -    return send_file(req, get_repo(backend, mat).get_named_file(path),
> -                     'text/plain')
> +    return send_file(req, contents, 'text/plain')
>
>
>  def get_loose_object(req, backend, mat):
> @@ -143,18 +147,16 @@ def get_loose_object(req, backend, mat):
>
>  def get_pack_file(req, backend, mat):
>     req.cache_forever()
> -    path = _url_to_path(mat.group())
> +    contents, path = _contents_and_path_for_url(backend, mat)
>     logger.info('Sending pack file %s', path)
> -    return send_file(req, get_repo(backend, mat).get_named_file(path),
> -                     'application/x-git-packed-objects')
> +    return send_file(req, contents, 'application/x-git-packed-objects')
>
>
>  def get_idx_file(req, backend, mat):
>     req.cache_forever()
> -    path = _url_to_path(mat.group())
> +    contents, path = _contents_and_path_for_url(backend, mat)
>     logger.info('Sending pack file %s', path)
> -    return send_file(req, get_repo(backend, mat).get_named_file(path),
> -                     'application/x-git-packed-objects-toc')
> +    return send_file(req, contents,
> 'application/x-git-packed-objects-toc')
>
>
>  def get_info_refs(req, backend, mat):
> --
> 1.7.3.2.msysgit.0
>
>
> _______________________________________________
> Mailing list: https://launchpad.net/~dulwich-users
> Post to     : dulwich-users@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~dulwich-users
> More help   : https://help.launchpad.net/ListHelp
>