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

I pulled down #set to GenericSpreedSheet #1

Closed
wants to merge 7 commits into from

Conversation

pabloh
Copy link
Contributor

@pabloh pabloh commented May 4, 2012

I moved #set, #set_value and #set_type from Excelx, Openoffice and Excel2003XML to GenericSpreedSheet, since they were identical at each subclass.

def set_type(row,col,type,sheet=nil)
sheet = @default_value unless sheet
key = "#{row},#{col}"
@cell_type[sheet][[key]] = type
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this extra [] around key is a bug.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep

@Empact
Copy link
Contributor

Empact commented May 4, 2012

Thanks for the pull request. It's good to have help for cleaning up this library. :-)

@pabloh
Copy link
Contributor Author

pabloh commented May 9, 2012

@Empact, I added the deprecation warning you suggested and restore the original behavior for 'sheet' argument at #set, #set_value and #set_type.

Do you think I also add a commit, for testing that behavior you mentioned for GenericSpreadsheet#set, where a nil value for the 'sheet' argument should be overridden by a default value even when an explicit nil is passed?

@pabloh pabloh closed this Jun 9, 2012
simonoff pushed a commit that referenced this pull request Jan 5, 2015
stevendaniels pushed a commit that referenced this pull request Apr 15, 2016
get up-to-date with latest files
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.

2 participants