Dmitry Lisichkin wrote:
> I'll prefer remove that method at all but it used by SCM system.
> It is a very old code (r5204) and I don't know about it is actual for now days.

For example, source:tags/3.3.3/test/functional/repositories_git_controller_test.rb#L448 .

> It cause problem with ruby-debug-ide gem:
> https://github.com/ruby-debug/ruby-debug-ide/blob/ce3dbe0a76e11f34abf60740b7c88b23a1c3b5bf/lib/ruby-debug-ide/xml_printer.rb#L355

I don't know about ruby-debug-ide, but it seems this code can be removed if ruby-debug-ide does not support Ruby 1.8.7.
Because it comes from
https://github.com/ruby-debug/ruby-debug-ide/commit/81aa3f9b66b65f02cea84945c6109e1bbe39ad66 .

> Almost all non-ASCII strings in debugger shown as [BinaryData]
I think it is expected behavior of ruby-debug-ide on Ruby 1.8.
--------------------------------------------------------------------------------
Toshi MARUYAMA wrote:
> I don't know about ruby-debug-ide, but it seems this code can be removed if ruby-debug-ide does not support Ruby 1.8.7.
> Because it comes from
> https://github.com/ruby-debug/ruby-debug-ide/commit/81aa3f9b66b65f02cea84945c6109e1bbe39ad66.

I think that ruby-debug-ide IS support old Ruby

Toshi MARUYAMA wrote:
> I think it is expected behavior of ruby-debug-ide on Ruby 1.8.
And this is not a problem becouse there is not so much people that work with this fossil=)

Problem in redmine code
I only say that the monkey-patching of core Ruby classes is not a good idea.
I spent some time yesterday and found that is_binary_data? method was removed only in 2.0.0 with replacement syck-engine to psych-engine
https://github.com/ruby/ruby/blob/ruby_1_9_3/ext/syck/lib/syck/rubytypes.rb#L152
https://github.com/ruby/ruby/commit/5571c7315e118b339c6b6590e666dfda68a7327d

In psych that method was moved from String to service class (the only right way):
https://github.com/ruby/psych/commit/c9cd187d5aa8fa6607dd463b5f98a65483ae39ce

So.. There is a strange thing. Redmine retrieve method to String from old YAML-engine (sic!). It's not normal.
I don't know, maybe that should be self-method in @Redmine::Scm::Adapters::AbstractAdapter@ or in some new class or module:
<pre>
<code class="ruby">
def binary_data?(string)
  (string.count("^-~", "^\r\n").fdiv(string.size) > 0.3 || string.index("\x00")) unless string.empty?
end
</code>
</pre>
But it definitely should not be in String.
--------------------------------------------------------------------------------
Dmitry Lisichkin wrote:
> I don't know, maybe that should be self-method in @Redmine::Scm::Adapters::AbstractAdapter@ or in some new class or module:

This is patch in this way.

--------------------------------------------------------------------------------
Toshi MARUYAMA wrote:
> Dmitry Lisichkin wrote:
> > I don't know, maybe that should be self-method in @Redmine::Scm::Adapters::AbstractAdapter@ or in some new class or module:
>
> This is patch in this way.

Big thanks.
One thing: ScmData is not look as class when it have only one self-method. I think that should be a module.
And if we have ScmData constant then we can name this method just @binary?@
--------------------------------------------------------------------------------

--------------------------------------------------------------------------------

--------------------------------------------------------------------------------
Committed note-5 patch, thanks.
--------------------------------------------------------------------------------