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

Enhancing input file flexibility #55

Merged
merged 1 commit into from
Jul 27, 2024

Conversation

hiroaki
Copy link
Contributor

@hiroaki hiroaki commented Jul 1, 2024

The current GPX::GPXFile.new requires an instance of the File class for the gpx_file option when specifying a file, but I think this should be a bit more flexible.
For instance, Issue #34 expects StringIO and Tempfile to be loaded, and I agree.

I have checked the source of Nokogiri::XML::Document.parse for this modification. As a result, I thought it would be better if the parameters of GPX::GPXFile.new were the same as the API of Nokogiri::XML::Document.parse, but for compatibility reasons, I concluded that the implementation as shown in this PR would be better.
Incidentally, the idea of respond_to?(:read) is exactly what Nokogiri::XML::Document.parse does.

Implementing this PR facilitates the following scenarios:

  • Example: Reading from STDIN
$ cat tests/gpx_files/gpx10.gpx | ruby -Ilib -rgpx -e 'puts GPX::GPXFile.new(gpx_file: $stdin).tracks.first.name'
  • Example: Reading from OpenURI (where URI.open returns a Tempfile)
$ ruby -Ilib -ropen-uri -rgpx -e 'puts GPX::GPXFile.new(gpx_file: URI.open("https://raw.githubusercontent.com/dougfales/gpx/v1.1.1/tests/gpx_files/gpx10.gpx")).tracks.first.name'

I especially wanted to use it in the pipeline from STDIN, as in the first example.

@niborg
Copy link
Collaborator

niborg commented Jul 19, 2024

@hiroaki I'm just seeing this. This seems quite reasonable. I will evaluate in the next few days. Thank you for the contribution!

@niborg
Copy link
Collaborator

niborg commented Jul 27, 2024

Thank you!

@niborg niborg merged commit 6137069 into dougfales:master Jul 27, 2024
0 of 4 checks passed
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 this pull request may close these issues.

2 participants