Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why set the default value of path_info=None while it cannot be? #740

Closed
zhaoguixu opened this issue Jun 8, 2015 · 7 comments

Comments

Projects
None yet
4 participants
@zhaoguixu
Copy link
Contributor

commented Jun 8, 2015

As stated in issue #440, #857

def test_mapadapterpath_info_is_none(self):
        m = r.Map([])
        ma = m.bind('example.org', path_info=None)
        m.match(return_rule=True)   #==========> it will crash

I just inspect the code and have a question why set the default value of path_info as None when it cannot be?

def match(self, path_info=None, method=None, return_rule=False,
                query_args=None):
        ...
def bind(self, server_name, script_name=None, subdomain=None,
             url_scheme='http', default_method='GET', path_info=None,
             query_args=None):
        ...
@RonnyPfannschmidt

This comment has been minimized.

Copy link
Contributor

commented Jun 8, 2015

it can be given either in bind or match,

it has to be given in at least one, the one in match overriding the one given in bind

@zhaoguixu

This comment has been minimized.

Copy link
Contributor Author

commented Jun 8, 2015

As I think, it would be better to give a friendly raise when both None happened instead of a rough AttributeError confusing the user. Do you think so?

@RonnyPfannschmidt

This comment has been minimized.

Copy link
Contributor

commented Jun 8, 2015

i tested based on your example in a python shell an got a 404,
maybe i needs a rule to trigger the behavior

are you up for creating a small pull request including a test and fix?

@zhaoguixu

This comment has been minimized.

Copy link
Contributor Author

commented Jun 8, 2015

There is a typo in the test.

def test_mapadapterpath_info_is_none(self):
    m = r.Map([])
    ma = m.bind('example.org', path_info=None)
    ma.match(return_rule=True)   #==========> it will crash

...
File "/usr/local/lib/python2.7/dist-packages/werkzeug/routing.py", line 1382, in match
self.subdomain, path_info.lstrip('/'))
AttributeError: 'NoneType' object has no attribute 'lstrip'

In the PR, I just set the root path as the default when both bind and match are None.

zhaoguixu added a commit to zhaoguixu/werkzeug that referenced this issue Jun 8, 2015

zhaoguixu added a commit to zhaoguixu/werkzeug that referenced this issue Jun 8, 2015

@untitaker

This comment has been minimized.

Copy link
Member

commented Sep 7, 2015

@zhaoguixu You sent this PR to the wrong repository.

@zhaoguixu

This comment has been minimized.

Copy link
Contributor Author

commented Sep 9, 2015

So where is the right repo? Feel free to merge it if necessary.

@untitaker

This comment has been minimized.

Copy link
Member

commented Sep 9, 2015

You sent the PR against your own fork...

On 9 September 2015 13:07:36 CEST, Zhaogui notifications@github.com wrote:

So where is the right repo? Feel free to merge it if necessary.


Reply to this email directly or view it on GitHub:
#740 (comment)

Sent from my phone. Please excuse my brevity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.