Fix windows compatibility #114
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously I made the assumption that the filename would be the first part of the error message for a syntax error before the colon `:` while this is false, it is especially false on windows where every path starts with a drive letter and a colon such as `c://hello/world.rb`. This commit fixes that problem by checking for the file on disk and appending more parts to the string. This is not foolproof because an error message might be possible that someone is using a colon with a file that actually exists in front of it like: ``` /lol/foo.rb:imactuallyadirectory/realfile.rb ``` In this case if `/lol/foo.rb` existed then it would quit looking. While I'm noting this edge case for completeness, I am not currently handling it. I think case that would be rare. When SyntaxError is refactored in Ruby 3.2 then we don't have to rely on parsing out filenames anymore. Close #111
schneems
force-pushed
the
schneems/fix-windows
branch
from
1654813 to
9ae13c8
Compare
schneems
added a commit
that referenced
this pull request
Nov 20, 2021
As suggested in #119 (comment) we are now explicitly checking the type of the incoming error. Rather than raise a new error, I've chosen to emit a warning and re-raise the original error. This is a similar pattern to the case where we are not able to detect the filename from the error message. The goal is to provide visibility into the source of the behavior, but to break as few expectations as possible. The `DeadEnd.handle_error` should (ideally) never raise an error to the end user. The purpose of the library is to provide progressive error enhancement. If it fails it means we cannot make a better error, so therefore we should warn (so the user can fix or report the problem) and then fall back to the original error. I also added a test for the failure error condition where the filename cannot be pulled out of the SyntaxError message which was added in #114.
schneems
added a commit
that referenced
this pull request
Nov 20, 2021
As suggested in #119 (comment) we are now explicitly checking the type of the incoming error. Rather than raise a new error, I've chosen to emit a warning and re-raise the original error. This is a similar pattern to the case where we are not able to detect the filename from the error message. The goal is to provide visibility into the source of the behavior, but to break as few expectations as possible. The `DeadEnd.handle_error` should (ideally) never raise an error to the end user. The purpose of the library is to provide progressive error enhancement. If it fails it means we cannot make a better error, so therefore we should warn (so the user can fix or report the problem) and then fall back to the original error. I also added a test for the failure error condition where the filename cannot be pulled out of the SyntaxError message which was added in #114.
schneems
added a commit
that referenced
this pull request
Nov 21, 2021
* Add interface to require dead_end without core_ext Currently `dead_end` works by monkey patching `require` and friends. If someone wants to programmatically execute dead_end manually via `DeadEnd.handle_error` without monkey patches they can now do that by setting the environment variable `DISABLE_DEAD_END_CORE_EXT=1`. * Move api to `dead_end/api` instead of env var Because setting environment variables on systems may be difficult or inconsistent we are moving the API to be require based. Now anyone who wants to use dead_end without mutating `require` can do so via requiring `dead_end/api`. * Update lib/dead_end/api.rb Co-authored-by: Jake Zimmerman <zimmerman.jake@gmail.com> * Add explicit check for Syntax Error As suggested in #119 (comment) we are now explicitly checking the type of the incoming error. Rather than raise a new error, I've chosen to emit a warning and re-raise the original error. This is a similar pattern to the case where we are not able to detect the filename from the error message. The goal is to provide visibility into the source of the behavior, but to break as few expectations as possible. The `DeadEnd.handle_error` should (ideally) never raise an error to the end user. The purpose of the library is to provide progressive error enhancement. If it fails it means we cannot make a better error, so therefore we should warn (so the user can fix or report the problem) and then fall back to the original error. I also added a test for the failure error condition where the filename cannot be pulled out of the SyntaxError message which was added in #114. Co-authored-by: Jake Zimmerman <zimmerman.jake@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously I made the assumption that the filename would be the first part of the error message for a syntax error before the colon
:while this is false, it is especially false on windows where every path starts with a drive letter and a colon such asc://hello/world.rb.This commit fixes that problem by checking for the file on disk and appending more parts to the string.
This is not foolproof because an error message might be possible that someone is using a colon with a file that actually exists in front of it like:
In this case if
/lol/foo.rbexisted then it would quit looking.While I'm noting this edge case for completeness, I am not currently handling it. I think that case would be rare.
When SyntaxError is refactored in Ruby 3.2 then we don't have to rely on parsing out filenames anymore.
Close #111