Support an input file of pathnames

[lilole]

Thanks for this great project! I made this change because I have large project trees with mixed Ruby and non-Ruby files, and some of the Ruby files do not end in .rb.

I have a script that figures out all the Ruby files. There may be thousands of files to process, with long relative pathnames, so they simply won't fit on the command line. And even with xargs multiple passes would be required.

With this new option I can put all the pathnames into a file and process them all in 1 pass. It uses the short-form -L to coincide with the ctags tool.

I hope you find it useful.

[lilole]

Sorry I give up on making the tests work for Ruby 1.8.7.

I would vote that you drop support for 1.8.7 and add 2.4.1 to your CI config.

[lilole]

Cool. I'll add 1.8.7 to my rvm and try to clean up the tests. Cheers. -Dan

On Fri, Sep 1, 2017 at 11:27 AM, Mislav Marohnić notifications@github.com wrote:

@mislav requested changes on this pull request.

Thanks for your contribution! This is heading in the right direction.

Unfortunately, your PR will not convince us to drop 1.8 compatibility, especially because your implementation doesn't actually use any 1.9-only features. That means that I would like you to make the CI pass if possible 😊 If you get stuck then let me know and I can take over.

In lib/ripper-tags.rb https://github.com/tmm1/ripper-tags/pull/67#discussion_r136526807:

@@ -56,6 +57,9 @@ def self.option_parser(options) opts.on("--tag-relative[=OPTIONAL]", "Make file paths relative to the directory of the tag file") do |value| options.tag_relative = value != "no" end

• opts.on("-L", "--input-lines=FILE", "Read paths to process from given file; use - for stdin") do |file|

Should this say --input-files instead?

In lib/ripper-tags/data_reader.rb https://github.com/tmm1/ripper-tags/pull/67#discussion_r136527015:

@@ -67,7 +67,7 @@ def resolve_file(file, depth = 0, &block) elsif depth > 0 || File.exist?(file) file = clean_path(file) if depth == 0 yield file if include_file?(file)

• elsif

Nice catch

In lib/ripper-tags/data_reader.rb https://github.com/tmm1/ripper-tags/pull/67#discussion_r136527131:

@@ -81,8 +81,11 @@ def clean_path(file)

 def each_file(&block)
   return to_enum(__method__) unless block_given?

• options.files.each do |file|
• resolve_file(file, &block)
• if (in_file = options.input_file)

There is nothing gained by assigning options.input_file to in_file

In test/test_cli.rb https://github.com/tmm1/ripper-tags/pull/67#discussion_r136527530:

@@ -109,6 +110,12 @@ def test_extra_flag_modifiers assert_equal %w[b d e].to_set, options.extra_flags end

• def test_input_file
• test_input_io = Tempfile.open("test-ripper-tags")

There's no need to create a Tempfile here. Any string would do. We just need to check that -L was processed well.

In test/test_data_reader.rb https://github.com/tmm1/ripper-tags/pull/67#discussion_r136527910:

@@ -91,6 +91,45 @@ def with_default_encoding(name) end end

• def test_input_file
• test_inputs = %w[encoding.rb very/inter.rb]
• with_tempfile(test_inputs) do |test_input_io|
• test_input_io.close
• options = OpenStruct.new(:input_file => test_input_io.path)
• finder = RipperTags::FileFinder.new(options)
• assert_equal test_inputs, finder.each_file.to_a
• end
• end
• def test_input_file_as_stdin
• test_inputs = %w[encoding.rb very/inter.rb]
• with_tempfile(test_inputs) do |test_input_io|

There is no need for tempfile in stdin test case. Instead, $stdin can simply temporarily be replaced by a StringIO instance.

In test/test_data_reader.rb https://github.com/tmm1/ripper-tags/pull/67#discussion_r136527990:

• assert_equal test_inputs, finder.each_file.to_a
• ensure
• $stdin = orig_stdin
• end
• end
• end
• def with_tempfile(lines, &block)
• in_fixtures do
• test_input_io = Tempfile.new("test-ripper-tags")
• lines.each { |line| test_input_io.puts(line) }
• begin
• block.call(test_input_io)
• ensure
• test_input_io.close if ! test_input_io.closed?

if ! is unless

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/tmm1/ripper-tags/pull/67#pullrequestreview-60077918, or mute the thread https://github.com/notifications/unsubscribe-auth/AM7yhv9RIQM-fkT1Oy90Le2rR_KX68S2ks5seDB3gaJpZM4PJovB .

[lilole]

@mislav I could not get rvm to even install 1.8.7 on my system. See some details here: https://github.com/rvm/rvm/issues/3659#issuecomment-326677212

I addressed some PR comments. The code is yours, feel free to take over and make whatever changes you like. Cheers.

[lilole]

I think 1.8.7 tests would succeed now, but it appears that Travis can't install the ripper gem for the 1.8.7 CI env. Sorry I don't have experience with Travis to help debug this one.

[mislav]

I've dropped Ruby 1.8 compatibility in master because it doesn't seem I can make the "ripper" gem build on the new OS version that Travis workers have since upgraded to.

I've also refactored a code a bit and renamed the flag to --input-file=FILE (no plural). Please review!

[phtmgt]

So, 1.8.7 will not work or?