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

Remove obsoleted Ruby code. #527

Closed
wants to merge 1 commit into from
Closed

Conversation

sho-h
Copy link
Contributor

@sho-h sho-h commented Sep 27, 2017

RDoc::RubyToken::Symbol was parsed as Symbol class and its content is Integer.

To check this PR, see @classes_hash in RDoc::Store. It has only one Symbol class.
@classes_hash at rdoc 5.1.0 has two Symbol class like below.

{
 "Symbol" => <Symbol@NormalClass>,
 "RDoc::RubyToken::Symbol" => <RDoc::RubyToken::Symbol@Normalclass but its name will be "Symbol" and its content will be Integer's one>
}

@aycabta
Copy link
Member

aycabta commented Sep 27, 2017

This isn't related to #422 and just workaround for old Rubies earlier than 1.4 what don't have Symbol. And at new Rubies older than 1.5, !defined?(Symbol) returns false so the code Symbol = Integer isn't evaluated, and active Rubies at present (2.2, 2.3, 2.4) and trunk don't evaluate it too.

And the lib/rdoc/ruby_token.rb will be removed and it's unnecessary at 6.0 as described at #422.

RDoc::RubyToken::Symbol was parsed as Symbol class and its content is Integer.
@sho-h
Copy link
Contributor Author

sho-h commented Sep 27, 2017

Sorry, no relation. I removed issue reference.

and just workaround for old Rubies earlier than 1.4

wmm... I think just workaround, too. But I think we can delete this line already.

!defined?(Symbol) returns false so the code Symbol = Integer isn't evaluated, and active Rubies at present (2.2, 2.3, 2.4) and trunk don't evaluate it too.

I know !defined?(Symbol) returns false. But RDoc::Store has RDoc::RubyToken::Symbol.
RDoc::RubyToken::Symbol was parsed as Integer. And ts name was Symbol. :stopdoc: was ignoring?
See https://docs.ruby-lang.org/en/2.4.0/Symbol.html
rdoc create Symbol.html twice now. We can see correct Symbol.html only when ::Symbol output on second time.

I want fix stopdoc or take this workaround, and release 5.1.1.

@sho-h sho-h changed the title Remove obsoleted Ruby code. (see #422) Remove obsoleted Ruby code. Sep 27, 2017
@aycabta
Copy link
Member

aycabta commented Sep 27, 2017

I removed whole lib/rdoc/ruby_token.rb at #529 and outputed documents of CRuby don't change between before and after of it because the file isn't used already.

I want fix stopdoc or take this workaround, and release 5.1.1.

It's difficult because we don't use v5 branch and master branch is just for 6.0.0 now. RDoc 5 uses legacy lexical analyser what is forked from IRB and written 20 years ago, so RDoc 6 uses Ripper what is standard library of Ruby. Please check my presentation of RubyKaigi 2017 (by the by, I know that you ware in my audience).

It's hard to just understand parsing Ruby code of RDoc partially, and fixing bug is more heavy. You can try and I cheer it, but I guess that the problem of this issue is so complex case. Maybe, I'll fix the problem later. Please wait it.

@aycabta
Copy link
Member

aycabta commented Sep 27, 2017

Just in case, please check creating the broken document with RDoc of head of master branch, because you know a way to reproduce the bug.

And I want to add some tests for it. So I want to reproduce the bug surely. Please tell me the way to reproduce the bug.

@sho-h
Copy link
Contributor Author

sho-h commented Sep 28, 2017

I want fix stopdoc or take this workaround, and release 5.1.1.

It's difficult because we don't use v5 branch and master branch is just for 6.0.0 now.
...
It's hard to just understand parsing Ruby code of RDoc partially, and fixing bug is more heavy. You can try and I cheer it, but I guess that the problem of this issue is so complex case. Maybe, I'll fix the problem later. Please wait it.

OK. I'll take workaround on docs.ruby-lang.org.
Screenshot before taking workaround is below.

2017-09-28 9 13 43

And I want to add some tests for it. So I want to reproduce the bug surely. Please tell me the way to reproduce the bug.

Just parsing Ruby source. But the bug wasn't reproduced always. @classes_hash in RDoc::Store have symbols like below.

# pattern 1: ::Symbol, RDoc::RubyToken::Symbol order. Incorrect Symbol.html generated.
store.all_classes_and_modules
# => [<Symbol@NormalClass>, ..., <RDoc::RubyToken::Symbol@Normalclass>, ...]

# pattern2: RDoc::RubyToken::Symbol, ::Symbol order. Correct Symbol.html generated.
store.all_classes_and_modules
# => [<RDoc::RubyToken::Symbol@Normalclass>, ..., <Symbol@NormalClass>, ...]

Both Normalclass#name returns "Symbol". So sometimes incorrect Symbol.html generated.

@sho-h sho-h closed this Sep 28, 2017
@sho-h sho-h deleted the remove-1.4-code branch September 28, 2017 00:38
@aycabta
Copy link
Member

aycabta commented Sep 28, 2017

OK, I understand that is probabilistic or outside condition.

I think the Symbol = Integer code creates alias for class. So the reason of the problem is parsing RDoc code of CRuby. So I want your answer of a question; Did you remove the "obsoleted Ruby code" from RDoc inside CRuby, or, RDoc outside CRuby (gem)?

@sho-h
Copy link
Contributor Author

sho-h commented Sep 28, 2017

Did you remove the "obsoleted Ruby code" from RDoc inside CRuby, or, RDoc outside CRuby (gem)?

I want to remove obsoleted Ruby code from RDoc inside CRuby. :)

@aycabta
Copy link
Member

aycabta commented Sep 28, 2017

OK, thank you. I understand.

@aycabta
Copy link
Member

aycabta commented Sep 28, 2017

I checked all things about this problem. The #529 will be picked to Ruby core trunk soon by @hsbt. But, Ruby 2.2, 2.3 and 2.4 will not be backported if anyone don't do nothing.

By the way, 2.2 is under security maintenance, 2.3 and 2.4 are under normal maintenance. So I think you should send issue(s) to 2.3 and 2.4 at bugs.ruby-lang.org with a patch of this Pull Request, because Symbol is very important class but broken. You should explain the broken documents status there.

If you send the issue(s), please tell me it. You can use here for it. After that, I'll help you about RDoc internal matters there.

@sho-h
Copy link
Contributor Author

sho-h commented Sep 29, 2017

@aycabta Thanks a lot!! OK. I'll create issue at bugs.ruby-lang.org later.

@sho-h
Copy link
Contributor Author

sho-h commented Oct 9, 2017

@aycabta I created https://bugs.ruby-lang.org/issues/13990.

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

Successfully merging this pull request may close these issues.

None yet

2 participants