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

When given a filename, QuickenParser.parse generates FrozenError #2

Open
lindes opened this issue Dec 30, 2021 · 0 comments · May be fixed by #3
Open

When given a filename, QuickenParser.parse generates FrozenError #2

lindes opened this issue Dec 30, 2021 · 0 comments · May be fixed by #3

Comments

@lindes
Copy link

lindes commented Dec 30, 2021

Hi there... I don't know if you still care about this code, but I found it and it looks like it'll help me with something, so I'm taking a look. Getting started, though, I noticed something that I would have expected to work which doesn't... and it's an easy fix, so:

If I have code like:

QuickenParser.parse('/path/to/some.qfx')

I'll get an error something like*:

Traceback (most recent call last):
        3: from [ruby-install-path]/gems/quicken_parser-0.2.1/lib/quicken_parser.rb:9:in `parse'
        2: from [ruby-install-path]/gems/quicken_parser-0.2.1/lib/quicken_parser/parser.rb:13:in `parse'
        1: from [ruby-install-path]/gems/quicken_parser-0.2.1/lib/quicken_parser/parser.rb:85:in `normalize_line_endings!'
[ruby-install-path]/gems/quicken_parser-0.2.1/lib/quicken_parser/parser.rb:85:in `gsub!': can't modify frozen String (FrozenError)

I know this differs a bit from the usage documented in the README, but hopefully you'll agree it's a natural enough usage.

The fix I've come up with for myself, which you're welcome to incorporate (I may also follow this up with a pull request, in case that's easier), is expressed by the following patch:

diff --git a/lib/quicken_parser/parser.rb b/lib/quicken_parser/parser.rb
index d94c517..e95053c 100644
--- a/lib/quicken_parser/parser.rb
+++ b/lib/quicken_parser/parser.rb
@@ -4,7 +4,8 @@ require "time"
 module QuickenParser
   class Parser #:nodoc:
     def initialize(source)
-      @input = source.respond_to?(:read) ? source.read : source
+      @input = source.respond_to?(:read) ? source.read :
+        File.exists?(source) ? File.open(source).read : source
     end
 
     def parse

As a side-note, you may notice that the errors above indicate a gem version of 0.2.1, yet the code in this repo is labeled 0.2.0. I don't know where 0.2.1 came from, as I couldn't find it in any of the forks. @agilous's fork has a 0.2.0.1 , but it's not the same. And there are real differences between 0.2.0 and 0.2.1... I've created a branch called gem_installed-0.2.1 in my fork to capture it, in case you want to look. It's getting rid of some code you added, presumably intentionally, so I'm basing the PR on what's otherwise in this repo, but I'd be happy to do a version based on that change instead, if that's better.

PR to follow. Thanks for creating this!

  • error text modified slightly to obscure personal pathnames that I believe to be irrelevant.
lindes added a commit to lindes/quicken_parser that referenced this issue Dec 30, 2021
It seems natural to pass a filename to initialize, so if whatever we're
given is a file that exists, read its contents instead of using it as
string data directly.

(It occurs to me that we may want to duplicate the string otherwise, but
I'm unsure what other use cases might be present that I'm not thinking
of, so I'm leaving that out for now.)

Fixes francois#2

Note: Chosing to bump version to 0.2.2 because rubygems has a 0.2.1 from
somewhere that I couldn't find.

This change technically Copyright 2021 by David Lindes, and I agree to
license it under the same terms as the rest of this repository.
@lindes lindes linked a pull request Dec 30, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant