Skip to content
This repository has been archived by the owner on Jul 19, 2018. It is now read-only.

JS refactoring #91

Merged
merged 8 commits into from
Nov 19, 2012
Merged

JS refactoring #91

merged 8 commits into from
Nov 19, 2012

Conversation

tobie
Copy link
Owner

@tobie tobie commented Nov 12, 2012

This adds a bunch of tests and does a full re-write.

To run tests install mocha, then:

mocha -u tdd

console.log(r.os.patch); // -> null

console.log(r.os.tostring()); // -> "iOS 5.1"
console.log(r.os.toVersionString()); // -> "5.1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section appears to be a duplicate of the previous.

@Krinkle
Copy link
Contributor

Krinkle commented Nov 19, 2012

Would be nice to get this merged, I'm in need of a replacement for a certain jQuery plugin I used to use for userAgent detection would like to use ua-parser instead. But that depends on #66, which I won't rebase until you're ready with this (unless you are willing to merge mine first and rebase this, but since they are overlapping and implementing similar things in different ways, I assume not).

@tobie
Copy link
Owner Author

tobie commented Nov 19, 2012

@bluesmoon I'd like your advise on this. Adding parsing for the device and os as part of the same function adds a significant cost to this, and you were already worrying about perf implication earlier on. Is adding a function that does only UA parsing a good enough option in your book?

// -> 1
var r = uaParser.parse(navigator.userAgent);

console.log(r.ua.tostring()); // -> "Safari 5.0.1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Properties (including functions) are case sensitive. This should be toString (same for r.os.tostring).

@bluesmoon
Copy link
Collaborator

Yes, as long as the caller can choose whether to call parse or parseXX, the performance implications are not a concern.

tobie added a commit that referenced this pull request Nov 19, 2012
@tobie tobie merged commit 6d57311 into master Nov 19, 2012
skliffmueller pushed a commit to skliffmueller/ua-parser that referenced this pull request Oct 22, 2015
skliffmueller pushed a commit to skliffmueller/ua-parser that referenced this pull request Oct 22, 2015
Fix broken indentation in test file
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants