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

is STDIN.tty? really needed? #4

Open
Sparkboxx opened this issue Aug 31, 2015 · 4 comments
Open

is STDIN.tty? really needed? #4

Sparkboxx opened this issue Aug 31, 2015 · 4 comments

Comments

@Sparkboxx
Copy link
Member

In https://github.com/opener-project/language-identifier/blob/master/lib/opener/language_identifier/cli.rb#L72 a call is being made to see if STDIN.tty?

If true - input is set to nil
If false - input is set to STDIN.read

I don't immediately see the reason for this, so the question is: why?
It seems to be broken now, I get a "length" is not defined for nil error.

Did you consider maybe using ARGF.read so that the LanguageIdentifier works both with STDIN and with filenames?

@Sparkboxx
Copy link
Member Author

I added a commit with a potential solution in a separate branch. There might be a reason for the nil value of input, in which case this commit will probably break stuff.

@yorickpeterse
Copy link
Contributor

STDIN.tty? returns false if the command is run while piping input to it. This allows usage such as echo foo | language-identifier. Not checking for this would result in STDIN.read blocking in case of no input. Using ARGF would be better (since it indeed would support a file path), but blindly calling ARGF.read will still block (the same applies to ARGF.eof?).

In other words, yes, lets use ARGF, but the tty? check should remain (unless there's a better alternative).

@Sparkboxx
Copy link
Member Author

You could argue that being able to “type” a string and then on line termination get a language back is a feature not a bug:
Something like this:

if STDIN.tty?
  ARGF.each do |input|
    puts identifier.run(input)
  end
else
  puts identifier.run(STDIN.read)
end

Will work for all cases:

  • echo “some text” | language-identifier
  • language-identifier file1.txt
  • language-identifier file1.txt file2.txt
  • language-identifier

in the last case it will simply allow you to type something and on pressing return it analyses the language.

Do you encounter practical situations where you end up passing “nothing”. Since on empty string (“”) it will not block on STDIN.read we might be trying to solve a problem that is not there?

On 31 Aug 2015, at 22:27, Yorick Peterse notifications@github.com wrote:

STDIN.tty? returns false if the command is run while piping input to it. This allows usage such as echo foo | language-identifier. Not checking for this would result in STDIN.read blocking in case of no input. Using ARGF would be better (since it indeed would support a file path), but blindly calling ARGF.read will still block (the same applies to ARGF.eof?).


Reply to this email directly or view it on GitHub #4 (comment).

@yorickpeterse
Copy link
Contributor

  • echo “some text” | language-identifier
  • language-identifier file1.txt
  • language-identifier file1.txt file2.txt
  • language-identifier

All these cases look fine to me. I think currently just running language-identifier doesn't do anything useful any way.

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

No branches or pull requests

2 participants