Skip to content

Fixed to work with updated DBF library (last commit Aug 2nd 2018 ca2b… #47

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fixed to work with updated DBF library (last commit Aug 2nd 2018 ca2b… #47

wants to merge 2 commits into from

Conversation

Nazz78
Copy link

@Nazz78 Nazz78 commented Aug 6, 2018

Added proper ColumnType::Base according to updated DBF library? Works for me.

Nazz78 added 2 commits August 6, 2018 23:38
@Nazz78
Fixed to work with updated DBF library (last commit Aug 2nd 2018 ca2b…
2f0ce48 
…59e04bddb6e7d802f70040f57e29be666b05)
@Nazz78
Added option to read DBF with specified encoding.
5f77f89 
This fixes the problem with wrong encoding when dbf file is read and encoding is not specified. We eg. use UTF-8, which gives correct results  instead of some strange characters when non-ascii characters are used.
houndci-bot
houndci-bot reviewed Aug 12, 2018
View reviewed changes
lib/geo_ruby/shp4r/shp.rb
end

@dbf = Dbf::Reader.open(@file_root + '.dbf')
@dbf = Dbf::Reader.open(@file_root + '.dbf', nil, dbf_encoding)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

lib/geo_ruby/shp4r/dbf.rb
# Open DBF with specified encoding (it can also be nil and DBF reader
# will use what it finds in header).
def self.open(f, memo = nil, encoding)
new(f, memo, encoding)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Layout/IndentationWidth: Use 2 (not 8) spaces for indentation.

lib/geo_ruby/shp4r/dbf.rb
new(f)
# Open DBF with specified encoding (it can also be nil and DBF reader
# will use what it finds in header).
def self.open(f, memo = nil, encoding)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Layout/Tab: Tab detected.
Layout/IndentationConsistency: Inconsistent indentation detected.
Naming/UncommunicativeMethodParamName: Method parameter must be at least 3 characters long.

lib/geo_ruby/shp4r/dbf.rb
def self.open(f)
new(f)
# Open DBF with specified encoding (it can also be nil and DBF reader
# will use what it finds in header).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Layout/Tab: Tab detected.

lib/geo_ruby/shp4r/dbf.rb

def self.open(f)
new(f)
# Open DBF with specified encoding (it can also be nil and DBF reader
Copy link
Collaborator

Choose a reason for hiding this comment

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

Layout/Tab: Tab detected.

@kueda
Copy link
Contributor

kueda commented Oct 3, 2018

Do the specs pass for you, @Nazz78? I get a lot of failures, e.g.

  7) GeoRuby::Shp4r Write test_creation_multipoint
     Failure/Error: super(name, type, length, decimal, version, enc)
     
     ArgumentError:
       wrong number of arguments (given 6, expected 2)
     # /path/to/ruby/gems/dbf-3.1.3/lib/dbf/column_type.rb:11:in `initialize'
     # ./lib/geo_ruby/shp4r/dbf.rb:25:in `initialize'
     # ./spec/geo_ruby/shp4r/shp_spec.rb:226:in `new'
     # ./spec/geo_ruby/shp4r/shp_spec.rb:226:in `block (3 levels) in <top (required)>'

@Nazz78
Copy link
Author

Nazz78 commented Oct 5, 2018

@kueda I don't know. Can you please provide me with some link on how to set the specs up?

@kueda
Copy link
Contributor

kueda commented Oct 5, 2018

For me I just ran bundle to make sure I had all the deps and then ran rake. You could also run rspec.

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

Reviewers

@houndci-bot houndci-bot houndci-bot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Nazz78 @kueda @houndci-bot