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

fix: Escape - in Regexp ranges #226

Closed
wants to merge 3 commits into from
Closed

fix: Escape - in Regexp ranges #226

wants to merge 3 commits into from

Conversation

ixti
Copy link

@ixti ixti commented Jan 14, 2024

Bootsnap warns if it sees confusing regexp range like [a-b-_]. In general, it's better to escape dash in regexp ranges when it's supposed to mean literal dash.

This patch fixes warnings (when bootsnap is active):

/home/ixti/.gem/ruby/3.0.6/gems/lob-6.0.5/lib/openapi_client/models/bank_account.rb:260: warning: character class has '-' without escape
/home/ixti/.gem/ruby/3.0.6/gems/lob-6.0.5/lib/openapi_client/models/bank_account.rb:296: warning: character class has '-' without escape
/home/ixti/.gem/ruby/3.0.6/gems/lob-6.0.5/lib/openapi_client/models/bank_account.rb:395: warning: character class has '-' without escape
/home/ixti/.gem/ruby/3.0.6/gems/bootsnap-1.17.1/lib/bootsnap/compile_cache/iseq.rb:53: warning: character class has '-' without escape
/home/ixti/.gem/ruby/3.0.6/gems/bootsnap-1.17.1/lib/bootsnap/compile_cache/iseq.rb:53: warning: character class has '-' without escape
/home/ixti/.gem/ruby/3.0.6/gems/bootsnap-1.17.1/lib/bootsnap/compile_cache/iseq.rb:53: warning: character class has '-' without escape

Related-PR: #225

Bootsnap warns if it sees confusing regexp range like `[a-b-_]`.
In general, it's better to escape dash in regexp ranges when it's
supposed to mean literal dash.
@ixti
Copy link
Author

ixti commented Jan 14, 2024

I would also recommend moving regexps into constants - there's no point in compiling them on every method call.

ixti added 2 commits January 14, 2024 18:31
`Lob.configure { ... }` was removed at some point, thus specs needs
update.
@mrkaspa
Copy link
Contributor

mrkaspa commented Apr 25, 2024

I created this PR for solving the same #237

@ixti
Copy link
Author

ixti commented Apr 26, 2024

Yeah, we abandoned all hope with this gem - far too many issues with it and absolutely unresponsive devs:

  • auto-generated code is rubbish
  • no ruby-3.2, ruby-3.3 official support (or at least testing)

On top of that, Lob required us to upgrade the gem. The upgrade silently broke the code. Before the upgrade gem was correctly handling Tempfile passed as file: argument to Lob::LetterEditable (or whatever it was at that time). So we had code like:

Lob::LetterEditable.new({ file: tempfile, ... })

After the upgrade, the gem was seemingly working. No error, nothing. And we got billed for the letters that were sent. But the letters contained "#<File ...>" instead of the actual contents. The documentation says that file should be either PDF or HTML. One doesn't need to do quantum computing to clearly tell that #<File ...> is neither!

Lesson learned: don't use something created only to tick the box in the checklist of supported languages.

@ixti ixti closed this Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants