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

Fixed bug whereby absoluteTo failed on a root base #200

Merged
merged 1 commit into from Mar 31, 2015

Conversation

Projects
None yet
2 participants
@giltayar
Contributor

giltayar commented Mar 24, 2015

E.g. URI("tofile").absoluteTo("/file") should return "/tofile" but didn't.
Added test case
Gil Tayar
Fixed bug whereby absoluteTo failed on a root base
    E.g. URI("tofile").absoluteTo("/file") should return "/tofile" but didn't.
    Added test case
@@ -1863,6 +1863,7 @@
if (resolved.path().charAt(0) !== '/') {
basedir = base.directory();
basedir = basedir ? basedir : base.path().indexOf('/') === 0 ? '/' : '';

This comment has been minimized.

@rodneyrehm

rodneyrehm Mar 24, 2015

Member

that reads horribly (not that my code reads any better…)
wondering if the following makes more sense

// either basedir is a thing, or path begins with /, defaults to empty string
basedir || base.path().slice(0, 1) === '/' && '/' || '';
@rodneyrehm

rodneyrehm Mar 24, 2015

Member

that reads horribly (not that my code reads any better…)
wondering if the following makes more sense

// either basedir is a thing, or path begins with /, defaults to empty string
basedir || base.path().slice(0, 1) === '/' && '/' || '';

This comment has been minimized.

@giltayar

giltayar Mar 24, 2015

Contributor

Yes, I know. It somehow seems that there should be a cleaner way to do this... :-) But I couldn't find it (admittedly,

As for changing the ?: to ||&& - that's a question of style. My code is more "say what you mean", yours is more concise. I could go either way.

@giltayar

giltayar Mar 24, 2015

Contributor

Yes, I know. It somehow seems that there should be a cleaner way to do this... :-) But I couldn't find it (admittedly,

As for changing the ?: to ||&& - that's a question of style. My code is more "say what you mean", yours is more concise. I could go either way.

This comment has been minimized.

@rodneyrehm

rodneyrehm Mar 24, 2015

Member

yeah… this is personal preference, I agree.
since most of the code is using ternaries rather than "truthy-switches" (whatever they're called), I'd roll with your solution for now.

@rodneyrehm

rodneyrehm Mar 24, 2015

Member

yeah… this is personal preference, I agree.
since most of the code is using ternaries rather than "truthy-switches" (whatever they're called), I'd roll with your solution for now.

rodneyrehm added a commit that referenced this pull request Mar 31, 2015

Merge pull request #200 from giltayar/master
Fixed bug whereby absoluteTo failed on a root base

@rodneyrehm rodneyrehm merged commit 20d6b72 into medialize:master Mar 31, 2015

@rodneyrehm

This comment has been minimized.

Show comment
Hide comment
@rodneyrehm

rodneyrehm Mar 31, 2015

Member

released v1.15.0, thank you for your support!

Member

rodneyrehm commented Mar 31, 2015

released v1.15.0, thank you for your support!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment