From e85801445f278a81d1e3a2901c4093e8f9d3e38a Mon Sep 17 00:00:00 2001 From: Geremia Taglialatela Date: Mon, 15 May 2023 21:51:40 +0200 Subject: Fix Performance/StringIdentifierArgument offense --- .rubocop_todo.yml | 5 ----- lib/axlsx/drawing/pic.rb | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 2385ecb2..57bf7265 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -135,11 +135,6 @@ Performance/RegexpMatch: - 'lib/axlsx/workbook/workbook.rb' - 'lib/axlsx/workbook/worksheet/cell.rb' -# This cop supports safe autocorrection (--autocorrect). -Performance/StringIdentifierArgument: - Exclude: - - 'lib/axlsx/drawing/pic.rb' - # This cop supports safe autocorrection (--autocorrect). # Configuration parameters: EnforcedStyle. # SupportedStyles: separated, grouped diff --git a/lib/axlsx/drawing/pic.rb b/lib/axlsx/drawing/pic.rb index 77a051c3..fa08bd83 100644 --- a/lib/axlsx/drawing/pic.rb +++ b/lib/axlsx/drawing/pic.rb @@ -239,7 +239,7 @@ module Axlsx def swap_anchor(new_anchor) new_anchor.drawing.anchors.delete(new_anchor) @anchor.drawing.anchors[@anchor.drawing.anchors.index(@anchor)] = new_anchor - new_anchor.instance_variable_set "@object", @anchor.object + new_anchor.instance_variable_set :@object, @anchor.object @anchor = new_anchor end end -- cgit v1.2.3 From a0bef85fc877afe91e22558bac5e14e2f7f88dbe Mon Sep 17 00:00:00 2001 From: Geremia Taglialatela Date: Mon, 15 May 2023 22:12:05 +0200 Subject: Use `include?` and `find` for performance Fix a couple of performance RuboCop offenses in workbook --- .rubocop_todo.yml | 7 ------- lib/axlsx/workbook/workbook.rb | 4 ++-- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 2385ecb2..846974f6 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -106,11 +106,6 @@ Performance/CollectionLiteralInLoop: - 'lib/axlsx/package.rb' - 'lib/axlsx/workbook/worksheet/page_margins.rb' -# This cop supports unsafe autocorrection (--autocorrect-all). -Performance/Detect: - Exclude: - - 'lib/axlsx/workbook/workbook.rb' - # This cop supports safe autocorrection (--autocorrect). Performance/RedundantBlockCall: Exclude: @@ -121,7 +116,6 @@ Performance/RedundantMatch: Exclude: - 'lib/axlsx.rb' - 'lib/axlsx/stylesheet/color.rb' - - 'lib/axlsx/workbook/workbook.rb' # This cop supports safe autocorrection (--autocorrect). Performance/RedundantSplitRegexpArgument: @@ -132,7 +126,6 @@ Performance/RedundantSplitRegexpArgument: Performance/RegexpMatch: Exclude: - 'lib/axlsx/stylesheet/color.rb' - - 'lib/axlsx/workbook/workbook.rb' - 'lib/axlsx/workbook/worksheet/cell.rb' # This cop supports safe autocorrection (--autocorrect). diff --git a/lib/axlsx/workbook/workbook.rb b/lib/axlsx/workbook/workbook.rb index 3e4927aa..1a2f0488 100644 --- a/lib/axlsx/workbook/workbook.rb +++ b/lib/axlsx/workbook/workbook.rb @@ -395,8 +395,8 @@ module Axlsx # retrieve the cells from. e.g. range('Sheet1!A1:B2') will return an array of four cells [A1, A2, B1, B2] while range('Sheet1!A1') will return a single Cell. # @return [Cell, Array] def [](cell_def) - sheet_name = cell_def.split('!')[0] if cell_def.match('!') - worksheet = self.worksheets.select { |s| s.name == sheet_name }.first + sheet_name = cell_def.split('!')[0] if cell_def.include?('!') + worksheet = self.worksheets.find { |s| s.name == sheet_name } raise ArgumentError, 'Unknown Sheet' unless sheet_name && worksheet.is_a?(Worksheet) worksheet[cell_def.gsub(/.+!/, "")] -- cgit v1.2.3 From 4627bcce04ade9c17e1d0c169100a6288195f6ac Mon Sep 17 00:00:00 2001 From: Paul Kmiec Date: Sat, 13 May 2023 14:21:21 -0700 Subject: Cache col_ref to avoid allocations In cases with lots of rows, each column will ask for its col_ref which will always be the same for the same column_index. We can cache this to avoid lots of small string allocations. Modified `CellSerializer` to use `#col_ref` and `#row_ref` avoiding the string allocation caused by `#col_r` --- lib/axlsx.rb | 30 ++++++++++++++++++------- lib/axlsx/workbook/worksheet/cell_serializer.rb | 4 +++- test/tc_axlsx.rb | 8 ++++++- 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/lib/axlsx.rb b/lib/axlsx.rb index 4f15ba23..6e6a317d 100644 --- a/lib/axlsx.rb +++ b/lib/axlsx.rb @@ -112,21 +112,35 @@ module Axlsx # @note This follows the standard spreadsheet convention of naming columns A to Z, followed by AA to AZ etc. # @return [String] def self.col_ref(index) - chars = +'' - while index >= 26 do - index, char = index.divmod(26) - chars.prepend((char + 65).chr) - index -= 1 + # Every row will call this for each column / cell and so we can cache result and avoid lots of small object + # allocations. + @col_ref ||= {} + @col_ref[index] ||= begin + i = index + chars = +'' + while i >= 26 + i, char = i.divmod(26) + chars.prepend((char + 65).chr) + i -= 1 + end + chars.prepend((i + 65).chr) + chars.freeze + chars end - chars.prepend((index + 65).chr) - chars + end + + # converts the row index into string values. + # @note The spreadsheet rows are 1-based and the passed in index is 0-based, so we add 1. + # @return [String] + def self.row_ref(index) + (index + 1).to_s end # @return [String] The alpha(column)numeric(row) reference for this sell. # @example Relative Cell Reference # ws.rows.first.cells.first.r #=> "A1" def self.cell_r(c_index, r_index) - col_ref(c_index) << (r_index + 1).to_s + col_ref(c_index) + row_ref(r_index) end # Creates an array of individual cell references based on an excel reference range. diff --git a/lib/axlsx/workbook/worksheet/cell_serializer.rb b/lib/axlsx/workbook/worksheet/cell_serializer.rb index e1bdf728..2baa4271 100644 --- a/lib/axlsx/workbook/worksheet/cell_serializer.rb +++ b/lib/axlsx/workbook/worksheet/cell_serializer.rb @@ -10,7 +10,9 @@ module Axlsx # @param [String] str The string to apend serialization to. # @return [String] def to_xml_string(row_index, column_index, cell, str = +'') - str << '' if cell.value.nil? method = cell.type diff --git a/test/tc_axlsx.rb b/test/tc_axlsx.rb index 95c77d88..a70722ee 100644 --- a/test/tc_axlsx.rb +++ b/test/tc_axlsx.rb @@ -83,8 +83,14 @@ class TestAxlsx < Test::Unit::TestCase end end + def test_row_ref + assert_equal('1', Axlsx.row_ref(0)) + assert_equal('100', Axlsx.row_ref(99)) + end + def test_cell_r - # todo + assert_equal('A1', Axlsx.cell_r(0, 0)) + assert_equal('Z26', Axlsx.cell_r(25, 25)) end def test_range_to_a -- cgit v1.2.3 From 71b358f0c73c73f88275b4c1c89fdf5372281ada Mon Sep 17 00:00:00 2001 From: Paul Kmiec Date: Sat, 13 May 2023 14:51:41 -0700 Subject: Corrected rubocop offenses in lib/axlsx.rb / test/tc_axlsx.rb --- .rubocop_todo.yml | 23 ----------------------- lib/axlsx.rb | 46 ++++++++++++++++++++++----------------------- lib/axlsx/util/constants.rb | 2 ++ test/tc_axlsx.rb | 2 ++ 4 files changed, 27 insertions(+), 46 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 2385ecb2..bc046dff 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -20,7 +20,6 @@ Layout/FirstHashElementIndentation: Layout/HashAlignment: Exclude: - 'lib/axlsx/workbook/worksheet/border_creator.rb' - - 'test/tc_axlsx.rb' # This cop supports safe autocorrection (--autocorrect). Lint/AmbiguousOperatorPrecedence: @@ -69,7 +68,6 @@ Lint/NonLocalExitFromIterator: # Configuration parameters: IgnoreEmptyBlocks, AllowUnusedKeywordArguments. Lint/UnusedBlockArgument: Exclude: - - 'lib/axlsx.rb' - 'lib/axlsx/drawing/axes.rb' - 'lib/axlsx/workbook/worksheet/pivot_table.rb' - 'lib/axlsx/workbook/worksheet/sheet_view.rb' @@ -119,7 +117,6 @@ Performance/RedundantBlockCall: # This cop supports safe autocorrection (--autocorrect). Performance/RedundantMatch: Exclude: - - 'lib/axlsx.rb' - 'lib/axlsx/stylesheet/color.rb' - 'lib/axlsx/workbook/workbook.rb' @@ -266,7 +263,6 @@ Style/GuardClause: # Configuration parameters: AllowSplatArgument. Style/HashConversion: Exclude: - - 'lib/axlsx.rb' - 'lib/axlsx/workbook/worksheet/cell_serializer.rb' - 'lib/axlsx/workbook/worksheet/rich_text_run.rb' @@ -321,7 +317,6 @@ Style/LineEndConcatenation: # Configuration parameters: AllowMethodComparison. Style/MultipleComparison: Exclude: - - 'lib/axlsx.rb' - 'lib/axlsx/stylesheet/font.rb' - 'lib/axlsx/workbook/worksheet/cell.rb' - 'lib/axlsx/workbook/worksheet/rich_text_run.rb' @@ -449,7 +444,6 @@ Style/PercentLiteralDelimiters: # This cop supports safe autocorrection (--autocorrect). Style/PerlBackrefs: Exclude: - - 'lib/axlsx.rb' - 'test/workbook/worksheet/tc_sheet_protection.rb' # This cop supports unsafe autocorrection (--autocorrect-all). @@ -471,7 +465,6 @@ Style/QuotedSymbols: # SupportedStyles: compact, exploded Style/RaiseArgs: Exclude: - - 'lib/axlsx.rb' - 'lib/axlsx/package.rb' - 'lib/axlsx/util/zip_command.rb' - 'lib/axlsx/workbook/worksheet/border_creator.rb' @@ -490,7 +483,6 @@ Style/RedundantCondition: # This cop supports safe autocorrection (--autocorrect). Style/RedundantFileExtensionInRequire: Exclude: - - 'lib/axlsx.rb' - 'lib/axlsx/content_type/content_type.rb' - 'lib/axlsx/drawing/drawing.rb' - 'lib/axlsx/rels/relationships.rb' @@ -528,7 +520,6 @@ Style/RedundantParentheses: # This cop supports safe autocorrection (--autocorrect). Style/RedundantRegexpEscape: Exclude: - - 'lib/axlsx.rb' - 'lib/axlsx/workbook/worksheet/pivot_table.rb' - 'lib/axlsx/workbook/worksheet/table.rb' @@ -536,7 +527,6 @@ Style/RedundantRegexpEscape: # Configuration parameters: AllowMultipleReturnValues. Style/RedundantReturn: Exclude: - - 'lib/axlsx.rb' - 'lib/axlsx/package.rb' - 'lib/axlsx/stylesheet/styles.rb' - 'lib/axlsx/workbook/worksheet/worksheet.rb' @@ -552,13 +542,6 @@ Style/RegexpLiteral: Exclude: - 'lib/axlsx/workbook/worksheet/cell.rb' -# This cop supports safe autocorrection (--autocorrect). -# Configuration parameters: EnforcedStyle. -# SupportedStyles: implicit, explicit -Style/RescueStandardError: - Exclude: - - 'lib/axlsx.rb' - # This cop supports unsafe autocorrection (--autocorrect-all). # Configuration parameters: ConvertCodeThatCanStartToReturnNil, AllowedMethods, MaxChainLength. # AllowedMethods: present?, blank?, presence, try, try! @@ -578,7 +561,6 @@ Style/SingleLineMethods: # This cop supports unsafe autocorrection (--autocorrect-all). Style/SlicingWithRange: Exclude: - - 'lib/axlsx.rb' - 'lib/axlsx/drawing/area_chart.rb' - 'lib/axlsx/drawing/line_chart.rb' - 'lib/axlsx/workbook/worksheet/pivot_table.rb' @@ -655,11 +637,6 @@ Style/UnpackFirst: Exclude: - 'lib/axlsx/workbook/worksheet/sheet_protection.rb' -# This cop supports safe autocorrection (--autocorrect). -Style/WhileUntilDo: - Exclude: - - 'lib/axlsx.rb' - # This cop supports safe autocorrection (--autocorrect). Style/WhileUntilModifier: Exclude: diff --git a/lib/axlsx.rb b/lib/axlsx.rb index 6e6a317d..30683458 100644 --- a/lib/axlsx.rb +++ b/lib/axlsx.rb @@ -1,29 +1,29 @@ # frozen_string_literal: true require 'htmlentities' -require 'axlsx/version.rb' +require 'axlsx/version' require 'marcel' -require 'axlsx/util/simple_typed_list.rb' -require 'axlsx/util/constants.rb' -require 'axlsx/util/validators.rb' -require 'axlsx/util/accessors.rb' +require 'axlsx/util/simple_typed_list' +require 'axlsx/util/constants' +require 'axlsx/util/validators' +require 'axlsx/util/accessors' require 'axlsx/util/serialized_attributes' require 'axlsx/util/options_parser' require 'axlsx/util/mime_type_utils' require 'axlsx/util/buffered_zip_output_stream' require 'axlsx/util/zip_command' -require 'axlsx/stylesheet/styles.rb' +require 'axlsx/stylesheet/styles' -require 'axlsx/doc_props/app.rb' -require 'axlsx/doc_props/core.rb' -require 'axlsx/content_type/content_type.rb' -require 'axlsx/rels/relationships.rb' +require 'axlsx/doc_props/app' +require 'axlsx/doc_props/core' +require 'axlsx/content_type/content_type' +require 'axlsx/rels/relationships' -require 'axlsx/drawing/drawing.rb' -require 'axlsx/workbook/workbook.rb' -require 'axlsx/package.rb' +require 'axlsx/drawing/drawing' +require 'axlsx/workbook/workbook' +require 'axlsx/package' # required gems require 'nokogiri' require 'zip' @@ -35,9 +35,9 @@ require 'time' begin if Gem.loaded_specs.has_key?("axlsx_styler") - raise StandardError.new("Please remove `axlsx_styler` from your Gemfile, the associated functionality is now built-in to `caxlsx` directly.") + raise StandardError, "Please remove `axlsx_styler` from your Gemfile, the associated functionality is now built-in to `caxlsx` directly." end -rescue +rescue StandardError # Do nothing end @@ -53,7 +53,7 @@ module Axlsx # # Defining as a class method on Axlsx to refrain from monkeypatching Object for all users of this gem. def self.instance_values_for(object) - Hash[object.instance_variables.map { |name| [name.to_s[1..-1], object.instance_variable_get(name)] }] + object.instance_variables.to_h { |name| [name.to_s[1..], object.instance_variable_get(name)] } end # determines the cell range for the items provided @@ -105,7 +105,7 @@ module Axlsx row_index = (numbers_str.to_i - 1) - return [col_index, row_index] + [col_index, row_index] end # converts the column index into alphabetical values. @@ -147,9 +147,9 @@ module Axlsx # @param [String] range A cell range, for example A1:D5 # @return [Array] def self.range_to_a(range) - range.match(/^(\w+?\d+)\:(\w+?\d+)$/) - start_col, start_row = name_to_indices($1) - end_col, end_row = name_to_indices($2) + range =~ /^(\w+?\d+):(\w+?\d+)$/ + start_col, start_row = name_to_indices(::Regexp.last_match(1)) + end_col, end_row = name_to_indices(::Regexp.last_match(2)) (start_row..end_row).to_a.map do |row_num| (start_col..end_col).to_a.map do |col_num| cell_r(col_num, row_num) @@ -163,7 +163,7 @@ module Axlsx def self.camel(s = "", all_caps = true) s = s.to_s s = s.capitalize if all_caps - s.gsub(/_(.)/) { $1.upcase } + s.gsub(/_(.)/) { ::Regexp.last_match(1).upcase } end # returns the provided string with all invalid control charaters @@ -184,7 +184,7 @@ module Axlsx # @param [Object] value The value to process # @return [Object] def self.booleanize(value) - if value == true || value == false + if BOOLEAN_VALUES.include?(value) value ? 1 : 0 else value @@ -195,7 +195,7 @@ module Axlsx # @param [Hash] Hash to merge into # @param [Hash] Hash to be added def self.hash_deep_merge(first_hash, second_hash) - first_hash.merge(second_hash) do |key, this_val, other_val| + first_hash.merge(second_hash) do |_key, this_val, other_val| if this_val.is_a?(Hash) && other_val.is_a?(Hash) Axlsx.hash_deep_merge(this_val, other_val) else diff --git a/lib/axlsx/util/constants.rb b/lib/axlsx/util/constants.rb index 4697bb3a..af8d9702 100644 --- a/lib/axlsx/util/constants.rb +++ b/lib/axlsx/util/constants.rb @@ -413,4 +413,6 @@ module Axlsx # Numeric recognition NUMERIC_REGEX = /\A[+-]?\d+?\Z/.freeze + + BOOLEAN_VALUES = [true, false].freeze end diff --git a/test/tc_axlsx.rb b/test/tc_axlsx.rb index a70722ee..3c4b5f93 100644 --- a/test/tc_axlsx.rb +++ b/test/tc_axlsx.rb @@ -3,6 +3,7 @@ require 'tc_helper' class TestAxlsx < Test::Unit::TestCase + # rubocop:disable Layout/HashAlignment def setup_wide @wide_test_points = { "A3" => 0, @@ -15,6 +16,7 @@ class TestAxlsx < Test::Unit::TestCase "BZU3" => (2 * (26**2)) + (26 * 26) + 20 } end + # rubocop:enable Layout/HashAlignment def test_cell_range_empty_if_no_cell assert_equal("", Axlsx.cell_range([])) -- cgit v1.2.3 From 57865b2f08ae301f4d6815211fba59fefafcec33 Mon Sep 17 00:00:00 2001 From: Paul Kmiec Date: Sun, 14 May 2023 20:01:22 -0700 Subject: Also cache row_ref The `row_ref` method is called once for each column in a row and once at the row level. --- lib/axlsx.rb | 4 ++-- lib/axlsx/workbook/worksheet/row.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/axlsx.rb b/lib/axlsx.rb index 30683458..bf16afbd 100644 --- a/lib/axlsx.rb +++ b/lib/axlsx.rb @@ -125,7 +125,6 @@ module Axlsx end chars.prepend((i + 65).chr) chars.freeze - chars end end @@ -133,7 +132,8 @@ module Axlsx # @note The spreadsheet rows are 1-based and the passed in index is 0-based, so we add 1. # @return [String] def self.row_ref(index) - (index + 1).to_s + @row_ref ||= {} + @row_ref[index] ||= (index + 1).to_s.freeze end # @return [String] The alpha(column)numeric(row) reference for this sell. diff --git a/lib/axlsx/workbook/worksheet/row.rb b/lib/axlsx/workbook/worksheet/row.rb index 16116835..dc0320b7 100644 --- a/lib/axlsx/workbook/worksheet/row.rb +++ b/lib/axlsx/workbook/worksheet/row.rb @@ -89,7 +89,7 @@ module Axlsx # @param [String] str The string this rows xml will be appended to. # @return [String] def to_xml_string(r_index, str = +'') - serialized_tag('row', str, :r => r_index + 1) do + serialized_tag('row', str, :r => Axlsx.row_ref(r_index)) do each_with_index { |cell, c_index| cell.to_xml_string(r_index, c_index, str) } end end -- cgit v1.2.3 From 1413f9be2cef8e0cc77113b94d1d0e4a7afc8ed6 Mon Sep 17 00:00:00 2001 From: Paul Kmiec Date: Tue, 2 May 2023 02:02:16 -0700 Subject: Treat escape_formulas similar to type, style, formula_value This avoid parse_options doing anything which can be expensive if it happens for each cell. --- lib/axlsx/workbook/worksheet/cell.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/axlsx/workbook/worksheet/cell.rb b/lib/axlsx/workbook/worksheet/cell.rb index 929a7c28..74ca7360 100644 --- a/lib/axlsx/workbook/worksheet/cell.rb +++ b/lib/axlsx/workbook/worksheet/cell.rb @@ -45,9 +45,10 @@ module Axlsx self.style = val unless val.nil? || val == 0 val = options.delete(:formula_value) self.formula_value = val unless val.nil? + val = options.delete(:escape_formulas) + self.escape_formulas = val.nil? ? row.worksheet.escape_formulas : val parse_options(options) - self.escape_formulas = row.worksheet.escape_formulas if escape_formulas.nil? self.value = value value.cell = self if contains_rich_text? -- cgit v1.2.3 From d18d7f9696211a0ed899153ce4d8ae9cb9c15b6d Mon Sep 17 00:00:00 2001 From: Paul Kmiec Date: Fri, 12 May 2023 11:08:06 -0700 Subject: Fix safe rubocop offenses in Axslx::Cell There are 2 offenses left but they would be breaking backwards compatibility. --- .rubocop_todo.yml | 13 ------------- lib/axlsx/workbook/worksheet/cell.rb | 20 +++++++++----------- 2 files changed, 9 insertions(+), 24 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 2385ecb2..2739661d 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -58,7 +58,6 @@ Lint/DisjunctiveAssignmentInConstructor: # Configuration parameters: IgnoreLiteralBranches, IgnoreConstantBranches. Lint/DuplicateBranch: Exclude: - - 'lib/axlsx/workbook/worksheet/cell.rb' - 'lib/axlsx/workbook/worksheet/merged_cells.rb' Lint/NonLocalExitFromIterator: @@ -133,7 +132,6 @@ Performance/RegexpMatch: Exclude: - 'lib/axlsx/stylesheet/color.rb' - 'lib/axlsx/workbook/workbook.rb' - - 'lib/axlsx/workbook/worksheet/cell.rb' # This cop supports safe autocorrection (--autocorrect). Performance/StringIdentifierArgument: @@ -323,7 +321,6 @@ Style/MultipleComparison: Exclude: - 'lib/axlsx.rb' - 'lib/axlsx/stylesheet/font.rb' - - 'lib/axlsx/workbook/worksheet/cell.rb' - 'lib/axlsx/workbook/worksheet/rich_text_run.rb' - 'test/workbook/tc_workbook_view.rb' @@ -383,7 +380,6 @@ Style/NumericPredicate: - 'lib/axlsx/stylesheet/font.rb' - 'lib/axlsx/util/validators.rb' - 'lib/axlsx/workbook/workbook.rb' - - 'lib/axlsx/workbook/worksheet/cell.rb' - 'lib/axlsx/workbook/worksheet/sheet_pr.rb' - 'lib/axlsx/workbook/worksheet/worksheet.rb' @@ -425,7 +421,6 @@ Style/ParenthesesAroundCondition: Exclude: - 'lib/axlsx/stylesheet/font.rb' - 'lib/axlsx/util/validators.rb' - - 'lib/axlsx/workbook/worksheet/cell.rb' - 'lib/axlsx/workbook/worksheet/rich_text_run.rb' # This cop supports safe autocorrection (--autocorrect). @@ -545,13 +540,6 @@ Style/RedundantReturn: Style/RedundantSelf: Enabled: false -# This cop supports safe autocorrection (--autocorrect). -# Configuration parameters: EnforcedStyle, AllowInnerSlashes. -# SupportedStyles: slashes, percent_r, mixed -Style/RegexpLiteral: - Exclude: - - 'lib/axlsx/workbook/worksheet/cell.rb' - # This cop supports safe autocorrection (--autocorrect). # Configuration parameters: EnforcedStyle. # SupportedStyles: implicit, explicit @@ -620,7 +608,6 @@ Style/SymbolProc: - 'lib/axlsx/drawing/drawing.rb' - 'lib/axlsx/stylesheet/styles.rb' - 'lib/axlsx/workbook/workbook.rb' - - 'lib/axlsx/workbook/worksheet/cell.rb' - 'lib/axlsx/workbook/worksheet/worksheet.rb' - 'lib/axlsx/workbook/worksheet/worksheet_hyperlinks.rb' diff --git a/lib/axlsx/workbook/worksheet/cell.rb b/lib/axlsx/workbook/worksheet/cell.rb index 74ca7360..37b8e205 100644 --- a/lib/axlsx/workbook/worksheet/cell.rb +++ b/lib/axlsx/workbook/worksheet/cell.rb @@ -42,7 +42,7 @@ module Axlsx self.type = type unless type == :string val = options.delete(:style) - self.style = val unless val.nil? || val == 0 + self.style = val unless val.nil? || val.zero? val = options.delete(:formula_value) self.formula_value = val unless val.nil? val = options.delete(:escape_formulas) @@ -291,7 +291,7 @@ module Axlsx # @see u def u=(v) - v = :single if (v == true || v == 1 || v == :true || v == 'true') + v = :single if [true, 1, :true, 'true'].include?(v) set_run_style :validate_cell_u, :u, v end @@ -354,7 +354,7 @@ module Axlsx # @example Absolute Cell Reference # ws.rows.first.cells.first.r #=> "$A$1" def r_abs - "$#{r.match(%r{([A-Z]+)([0-9]+)})[1, 2].join('$')}" + "$#{r.match(/([A-Z]+)([0-9]+)/)[1, 2].join('$')}" end # @return [Integer] The cellXfs item index applied to this cell. @@ -379,7 +379,7 @@ module Axlsx start, stop = if target.is_a?(String) [self.r, target] elsif target.is_a?(Cell) - Axlsx.sort_cells([self, target]).map { |c| c.r } + Axlsx.sort_cells([self, target]).map(&:r) end self.row.worksheet.merge_cells "#{start}:#{stop}" unless stop.nil? end @@ -509,13 +509,11 @@ module Axlsx :time elsif v.is_a?(TrueClass) || v.is_a?(FalseClass) :boolean - elsif v.to_s =~ Axlsx::NUMERIC_REGEX && v.respond_to?(:to_i) + elsif v.respond_to?(:to_i) && v.to_s =~ Axlsx::NUMERIC_REGEX :integer - elsif v.to_s =~ Axlsx::SAFE_FLOAT_REGEX && v.respond_to?(:to_f) + elsif v.respond_to?(:to_f) && (v.to_s =~ Axlsx::SAFE_FLOAT_REGEX || ((matchdata = v.to_s.match(MAYBE_FLOAT_REGEX)) && (Float::MIN_10_EXP..Float::MAX_10_EXP).cover?(matchdata[:exp].to_i))) :float - elsif (matchdata = v.to_s.match(MAYBE_FLOAT_REGEX)) && (Float::MIN_10_EXP..Float::MAX_10_EXP).cover?(matchdata[:exp].to_i) && v.respond_to?(:to_f) - :float - elsif v.to_s =~ Axlsx::ISO_8601_REGEX + elsif Axlsx::ISO_8601_REGEX.match?(v.to_s) :iso_8601 elsif v.is_a? RichText :richtext @@ -533,14 +531,14 @@ module Axlsx case type when :date - self.style = STYLE_DATE if self.style == 0 + self.style = STYLE_DATE if self.style.zero? if !v.is_a?(Date) && v.respond_to?(:to_date) v.to_date else v end when :time - self.style = STYLE_DATE if self.style == 0 + self.style = STYLE_DATE if self.style.zero? if !v.is_a?(Time) && v.respond_to?(:to_time) v.to_time else -- cgit v1.2.3 From e57d67b31d0edbdf3ffe1f2e16f1c262e1b48c4b Mon Sep 17 00:00:00 2001 From: Paul Kmiec Date: Sat, 13 May 2023 09:55:49 -0700 Subject: Using `between?` is more efficient than `cover?` Co-authored-by: Geremia Taglialatela --- lib/axlsx/workbook/worksheet/cell.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/axlsx/workbook/worksheet/cell.rb b/lib/axlsx/workbook/worksheet/cell.rb index 37b8e205..10818f5c 100644 --- a/lib/axlsx/workbook/worksheet/cell.rb +++ b/lib/axlsx/workbook/worksheet/cell.rb @@ -511,7 +511,7 @@ module Axlsx :boolean elsif v.respond_to?(:to_i) && v.to_s =~ Axlsx::NUMERIC_REGEX :integer - elsif v.respond_to?(:to_f) && (v.to_s =~ Axlsx::SAFE_FLOAT_REGEX || ((matchdata = v.to_s.match(MAYBE_FLOAT_REGEX)) && (Float::MIN_10_EXP..Float::MAX_10_EXP).cover?(matchdata[:exp].to_i))) + elsif v.respond_to?(:to_f) && (v.to_s =~ Axlsx::SAFE_FLOAT_REGEX || ((matchdata = v.to_s.match(MAYBE_FLOAT_REGEX)) && matchdata[:exp].to_i.between?(Float::MIN_10_EXP, Float::MAX_10_EXP))) :float elsif Axlsx::ISO_8601_REGEX.match?(v.to_s) :iso_8601 -- cgit v1.2.3 From af8360755fe21f4e9d30e943ba0b2be3c0128d28 Mon Sep 17 00:00:00 2001 From: Paul Kmiec Date: Sat, 13 May 2023 10:18:50 -0700 Subject: Improve boolean validation constants (i.e. Axlsx::VALID_BOOLEAN_VALUES) Added VALID_BOOLEAN_TRUE_VALUES and VALID_BOOLEAN_FALSE_VALUES so that those can be re-used in other placed and have the same notion of what a valid boolean value is. For example, we can use the true values in `Cell#u=`. Additionally, since validate_boolean / BOOLEAN_VALIDATOR are invoked so frequently, putting the likely values at the front can actually make a non-trivial difference. Since VALID_BOOLEAN_VALUES is derived from VALID_BOOLEAN_TRUE_VALUES and VALID_BOOLEAN_FALSE_VALUES, we use `Array#zip` to still end up with good order. --- lib/axlsx/util/validators.rb | 6 ++++-- lib/axlsx/workbook/worksheet/cell.rb | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/axlsx/util/validators.rb b/lib/axlsx/util/validators.rb index 0004f51a..752ffc76 100644 --- a/lib/axlsx/util/validators.rb +++ b/lib/axlsx/util/validators.rb @@ -106,8 +106,10 @@ module Axlsx DataTypeValidator.validate :signed_int, Integer, v end - VALID_BOOLEAN_CLASSES = [String, Integer, Symbol, TrueClass, FalseClass].freeze - VALID_BOOLEAN_VALUES = [0, 1, 'true', 'false', :true, :false, true, false, '0', '1'].freeze + VALID_BOOLEAN_CLASSES = [TrueClass, FalseClass, Integer, String, Symbol].freeze + VALID_BOOLEAN_TRUE_VALUES = [true, 1, '1', 'true', :true].freeze + VALID_BOOLEAN_FALSE_VALUES = [false, 0, '0', 'false', :false].freeze + VALID_BOOLEAN_VALUES = VALID_BOOLEAN_TRUE_VALUES.zip(VALID_BOOLEAN_FALSE_VALUES).flatten.freeze BOOLEAN_VALIDATOR = lambda { |arg| VALID_BOOLEAN_VALUES.include?(arg) } # Requires that the value is a form that can be evaluated as a boolean in an xml document. diff --git a/lib/axlsx/workbook/worksheet/cell.rb b/lib/axlsx/workbook/worksheet/cell.rb index 10818f5c..8afacee1 100644 --- a/lib/axlsx/workbook/worksheet/cell.rb +++ b/lib/axlsx/workbook/worksheet/cell.rb @@ -291,7 +291,7 @@ module Axlsx # @see u def u=(v) - v = :single if [true, 1, :true, 'true'].include?(v) + v = :single if VALID_BOOLEAN_TRUE_VALUES.include?(v) set_run_style :validate_cell_u, :u, v end -- cgit v1.2.3 From 447b2522bc4a06c8436e708eae40cf7244dc0ec4 Mon Sep 17 00:00:00 2001 From: Paul Kmiec Date: Sat, 13 May 2023 11:00:38 -0700 Subject: Only define @escape_formulas in cell if it is different from worksheet The benchmarks showed that validate_boolean is called 200_005 times and almost all of those are to validate escape_formulas passed into cell. In this commit the worksheet does not pass in its escape_formulas value, avoiding validate_boolean, and instead the cell asks the worksheet for value when needed. Now validate_boolean is called 5 times in benchmarks. --- lib/axlsx/workbook/worksheet/cell.rb | 6 ++++-- lib/axlsx/workbook/worksheet/worksheet.rb | 1 - 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/axlsx/workbook/worksheet/cell.rb b/lib/axlsx/workbook/worksheet/cell.rb index 8afacee1..b44656e3 100644 --- a/lib/axlsx/workbook/worksheet/cell.rb +++ b/lib/axlsx/workbook/worksheet/cell.rb @@ -46,7 +46,7 @@ module Axlsx val = options.delete(:formula_value) self.formula_value = val unless val.nil? val = options.delete(:escape_formulas) - self.escape_formulas = val.nil? ? row.worksheet.escape_formulas : val + self.escape_formulas = val unless val.nil? parse_options(options) @@ -146,7 +146,9 @@ module Axlsx # Allowing user-generated data to be interpreted as formulas is a security risk. # See https://www.owasp.org/index.php/CSV_Injection for details. # @return [Boolean] - attr_reader :escape_formulas + def escape_formulas + defined?(@escape_formulas) ? @escape_formulas : row.worksheet.escape_formulas + end # Sets whether to treat values starting with an equals sign as formulas or as literal strings. # @param [Boolean] value The value to set. diff --git a/lib/axlsx/workbook/worksheet/worksheet.rb b/lib/axlsx/workbook/worksheet/worksheet.rb index 960f33b3..21c53409 100644 --- a/lib/axlsx/workbook/worksheet/worksheet.rb +++ b/lib/axlsx/workbook/worksheet/worksheet.rb @@ -429,7 +429,6 @@ module Axlsx # Allowing user generated data to be interpreted as formulas can be dangerous # (see https://www.owasp.org/index.php/CSV_Injection for details). def add_row(values = [], options = {}) - options[:escape_formulas] = escape_formulas if options[:escape_formulas].nil? row = Row.new(self, values, options) update_column_info row, options.delete(:widths) yield row if block_given? -- cgit v1.2.3 From 1201ef7145463a14d263a093d7d53a2f235e48e0 Mon Sep 17 00:00:00 2001 From: Paul Kmiec Date: Mon, 15 May 2023 17:04:59 -0700 Subject: Remove ability to set `u=` to true in favor of :single The `u=` would convert `true` to `:single` for backwards compatibility. However, it is more explicit to set it to `:single` or one of the other underline options instead of relying on the conversion. --- CHANGELOG.md | 1 + lib/axlsx/util/validators.rb | 4 +--- lib/axlsx/workbook/worksheet/cell.rb | 4 +--- test/workbook/worksheet/tc_cell.rb | 2 +- 4 files changed, 4 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 19f5d742..92d3396f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ CHANGELOG - Drop support for Ruby versions < 2.6 - Added frozen string literals - Fix `SimpleTypedList#to_a` and `SimpleTypedList#to_ary` returning the internal list instance + - Remove ability to set `u=` to true in favor of using :single or one of the other underline options - **April.23.23**: 3.4.1 - [PR #209](https://github.com/caxlsx/caxlsx/pull/209) - Revert characters other than `=` being considered as formulas. diff --git a/lib/axlsx/util/validators.rb b/lib/axlsx/util/validators.rb index 752ffc76..e9cf13b6 100644 --- a/lib/axlsx/util/validators.rb +++ b/lib/axlsx/util/validators.rb @@ -107,9 +107,7 @@ module Axlsx end VALID_BOOLEAN_CLASSES = [TrueClass, FalseClass, Integer, String, Symbol].freeze - VALID_BOOLEAN_TRUE_VALUES = [true, 1, '1', 'true', :true].freeze - VALID_BOOLEAN_FALSE_VALUES = [false, 0, '0', 'false', :false].freeze - VALID_BOOLEAN_VALUES = VALID_BOOLEAN_TRUE_VALUES.zip(VALID_BOOLEAN_FALSE_VALUES).flatten.freeze + VALID_BOOLEAN_VALUES = [true, false, 1, 0, '1', '0', 'true', 'false', :true, :false].freeze BOOLEAN_VALIDATOR = lambda { |arg| VALID_BOOLEAN_VALUES.include?(arg) } # Requires that the value is a form that can be evaluated as a boolean in an xml document. diff --git a/lib/axlsx/workbook/worksheet/cell.rb b/lib/axlsx/workbook/worksheet/cell.rb index b44656e3..cbf55482 100644 --- a/lib/axlsx/workbook/worksheet/cell.rb +++ b/lib/axlsx/workbook/worksheet/cell.rb @@ -285,15 +285,13 @@ module Axlsx def extend=(v) set_run_style :validate_boolean, :extend, v; end # The inline underline property for the cell. - # It must be one of :none, :single, :double, :singleAccounting, :doubleAccounting, true + # It must be one of :none, :single, :double, :singleAccounting, :doubleAccounting # @return [Boolean] # @return [String] - # @note true is for backwards compatability and is reassigned to :single attr_reader :u # @see u def u=(v) - v = :single if VALID_BOOLEAN_TRUE_VALUES.include?(v) set_run_style :validate_cell_u, :u, v end diff --git a/test/workbook/worksheet/tc_cell.rb b/test/workbook/worksheet/tc_cell.rb index a534a8dc..fef23ede 100644 --- a/test/workbook/worksheet/tc_cell.rb +++ b/test/workbook/worksheet/tc_cell.rb @@ -567,7 +567,7 @@ class TestCell < Test::Unit::TestCase def test_to_xml # TODO: This could use some much more stringent testing related to the xml content generated! @ws.add_row [Time.now, Date.today, true, 1, 1.0, "text", "=sum(A1:A2)", "2013-01-13T13:31:25.123"] - @ws.rows.last.cells[5].u = true + @ws.rows.last.cells[5].u = :single schema = Nokogiri::XML::Schema(File.open(Axlsx::SML_XSD)) doc = Nokogiri::XML(@ws.to_xml_string) -- cgit v1.2.3