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 warning about unused local variable. #24

Closed
wants to merge 1 commit into from

Conversation

tom025
Copy link

@tom025 tom025 commented Jan 15, 2014

This removes the warning that was occurring by passing the HTMLFormatter#binding to a erb template without using that variable in the method.

The binding given to the erb template is that of a basic object that has no methods or variables apart from accessor methods. This hides the internals ofHTMLFormatter from the ERB template.

This change was tested locally against the cucumber-ruby-core project.

Please consider a 0.8.1 release as this warning has propagated down to other projects that depend on simplecov-html.

This removes the warning that was occuring by passing the `HTMLFormatter#binding`
to a erb template without using that variable in the method.

The binding given to the erb template is that of a basic object that has no
methods or variables apart from accessor methods. This hides the internals of
HTMLFormatter from the ERB template.

This change was tested locally against the cucumber-ruby-core project.

Please consider a 0.8.1 release as this warning has propergated down to other
projects that depend on simplecov-html.
@tom025
Copy link
Author

tom025 commented Feb 21, 2014

Any news on this?

@deivid-rodriguez
Copy link
Collaborator

👍

@@ -55,8 +55,21 @@ def formatted_source_file(source_file)
# Returns a table containing the given source files
def formatted_file_list(title, source_files)
title_id = title.gsub(/^[^a-zA-Z]+/, '').gsub(/[^a-zA-Z0-9\-\_]/, '')
title_id # Ruby will give a warning when we do not use this except via the binding :( FIXME
template('file_list').result(binding)
data = FileListData.new(title, title_id, source_files)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like overkill, no?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the downside of simplecov being a rather low-level tool - I've had plenty of reports about ruby warnings all around and yes, while it is incredibly silly to have this workaround in place, people seem to want warning-free libraries. Also, this particular piece broke the gem when I got a PR that removed the assignment and then in the vie the variable didn't exist :/

On 3. August 2014 14:39:49 MESZ, Benjamin Fleischer notifications@github.com wrote:

@@ -55,8 +55,21 @@ def formatted_source_file(source_file)

Returns a table containing the given source files

def formatted_file_list(title, source_files)
title_id = title.gsub(/^[^a-zA-Z]+/,
'').gsub(/[^a-zA-Z0-9-_]/, '')

  • title_id # Ruby will give a warning when we do not use this
    except via the binding :( FIXME
  • template('file_list').result(binding)
  • data = FileListData.new(title, title_id, source_files)

This seems like overkill, no?


Reply to this email directly or view it on GitHub:
https://github.com/colszowka/simplecov-html/pull/24/files#r15734293

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, having played with it a bit, I suppose this is the smallest change that fixes it. I started building a Template class and...

@bf4
Copy link
Collaborator

bf4 commented Aug 3, 2014

@tom025 Wanna be added as a collaborator? I'm okay to merge.. though I'd like to add this to a CHANGLEOG somewhere...

@robertgrimm
Copy link

This warning can also be fixed by doing a double assignment which is what rdoc does to fix this same warning when using ERB and binding.

I suggest doing the following, along with a suitable comment as to why:

title_id = title_id = title.gsub(/^[^a-zA-Z]+/, '').gsub(/[^a-zA-Z0-9\-\_]/, '')

Really simple and also fixes the "possibly useless use of a variable in void context" warning that exists with at least Ruby 2.1 (possibly other versions) with the current fix of simply having a "title_id" line after the assignment.

@deivid-rodriguez
Copy link
Collaborator

This can be closed now...

@sferik sferik closed this Dec 6, 2014
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.

6 participants