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

Compiler: fix require relative path resolution #7758

Merged
merged 2 commits into from
May 10, 2019

Conversation

asterite
Copy link
Member

@asterite asterite commented May 8, 2019

Fixes #7408

@asterite asterite added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler labels May 8, 2019
@asterite asterite force-pushed the bug/crystal-path-relative branch from f02897c to b12e734 Compare May 8, 2019 20:38
@asterite
Copy link
Member Author

Cool, I thought I implemented something wrong but it turned out we had a couple of invalid require that this PR now catches 💪

end
else
relative_filename = "#{relative_to}/#{filename}"
return unless relative_to.is_a?(String)
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this was in the original code, but why not have relative_to be typed to a String right in the signature?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. I will do this, I think it will simplify code (I thought it was being passed other things than String or Nil but it doesn't seem to be the case). Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, never mind. This can be done in a separate PR. If somebody wants to do it, please go ahead.

@asterite asterite force-pushed the bug/crystal-path-relative branch from cce8a54 to 2f53e5b Compare May 10, 2019 21:01
@asterite asterite merged commit bbffbe0 into crystal-lang:master May 10, 2019
@asterite asterite deleted the bug/crystal-path-relative branch May 10, 2019 21:02
@asterite asterite added this to the 0.29.0 milestone May 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"require" resolves nonsensical relative paths
3 participants