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

adding greedy space to model url splitter #284

Merged
merged 1 commit into from
Feb 21, 2013
Merged

adding greedy space to model url splitter #284

merged 1 commit into from
Feb 21, 2013

Conversation

onyxrev
Copy link
Contributor

@onyxrev onyxrev commented Feb 16, 2013

I am migrating a JMVC app over to CanJS and encountered strange DOM Exception 12 errors on model CRUD events. It took me a while to chase them down and discovered that the splitter employed in CanJS was failing to properly spit on my URLs because I used multiple spaces. I like all my routes to line up "just so" for better readability:

        findAll:  "GET    /api/" + AWT.api_version + "/advertisements.json",
        findOne : "GET    /api/" + AWT.api_version + "/advertisements/{id}.json",
        create :  "POST   /api/" + AWT.api_version + "/advertisements/advertisement.json",
        update :  "PUT    /api/" + AWT.api_version + "/advertisements/{id}.json",
        destroy : "DELETE /api/" + AWT.api_version + "/advertisements/{id}.json",

JMVC handled these routes okay.

This little fix just makes the splitter greedier to handle the extra spaces.

to avoid obscure DOM Exception 12 errors for people who include extra spaces in their URLs (better matches old JMVC behavior)
@daffl
Copy link
Contributor

daffl commented Feb 18, 2013

Looks good, thanks! Will merge it into 1.1.5.

daffl added a commit that referenced this pull request Feb 21, 2013
adding greedy space to model url splitter
@daffl daffl merged commit 5c3ce67 into canjs:master Feb 21, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants