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

[Bug Report]: Excel template, empty cells/rows get removed #86

Open
dail8859 opened this issue Jul 9, 2020 · 5 comments
Open

[Bug Report]: Excel template, empty cells/rows get removed #86

dail8859 opened this issue Jul 9, 2020 · 5 comments
Labels
P: High S: on hold The issue is on hold until further notice T: bug Functionality that does not work as intended/expected

Comments

@dail8859
Copy link

dail8859 commented Jul 9, 2020

Environment
Carbone Version: npm ls shows [email protected]
Node Version: v12.18.1
Browsers: Not applicable
Desktop OS: Windows Server 2012 R2

Expected Behaviour
In an excel template, empty cells or rows should not get removed

Actual Behaviour
When there are empty cells or rows, they are getting removed when rendering the template.

Steps to reproduce

  1. Create a new xlsx file called in.xlsx
  2. Go to cell C4 and put the string a
  3. Save the file
  4. Use carbone.render() (see code snippet below)
  5. Open the output file out.xlsx
  6. The a in cell C4 has been moved to cell A1

Screenshots
If applicable, add screenshots to help explain your problem.
image

Reproduction Link / Code example

const fs = require('fs');
const carbone = require('carbone');

// Data to inject
var data = {};

carbone.render('./in.xlsx', data, function(err, result){
	if (err) {
		return console.log(err);
	}

	fs.writeFileSync('out.xlsx', result);
});
@dail8859 dail8859 added the T: bug Functionality that does not work as intended/expected label Jul 9, 2020
@dail8859 dail8859 changed the title [Bug Report]: Excel template, empty cells get removed [Bug Report]: Excel template, empty cells/rows get removed Jul 9, 2020
@steevepay steevepay added P: High S: on hold The issue is on hold until further notice labels Jul 9, 2020
@steevepay
Copy link
Member

Hi @dail8859, thank you for reaching us!
The issue is known since Carbone supports XLSX files, it's an architecture choice but we plan to rework this part.
Until the fix, I advise you to create an ODS templates and export as a XLSX.

@steevepay steevepay mentioned this issue Jul 10, 2020
21 tasks
@dail8859
Copy link
Author

@steevepay Thanks for the information!

I can confirm that using ODS and converting it to XLSX does keep empty cells. However, our use case has an XLSX file that has custom formatting/drawings/etc that using plain LibreOffice just isn't able to convert to XLSX and keep everything successfully.

I look forward to seeing this issue addressed in the future. Thanks!

@nicoladefranceschi
Copy link

nicoladefranceschi commented Jan 17, 2021

Hi @steevepay,
I wrote a very ugly hack to fix this problem, maybe it can be helpful when fixing the xlsx file handling.

The idea is:
preprocessor
each time you find a row, replace the "r=" attribute to keep track of the delta (the difference from the current row index and the previous. Preserve also the information about the index of the row.
postprocessing (since there is not a postprocessing function, I needed to add the code in the builder.buildXML function)
each time you find a row, check if the previous row index is less/equal/greater than this row index

  • if previous row index < this row index, use the delta stored in the preprocessing stage
  • if previous row index >= this row index, we are inside a loop. The delta is the delta of the rows with the i+1 index, that has been removed and replaced with the repeating rows. To get that value, I saved in the spaceRight object, for each row, the delta of the next row.

Same for the columns!

Here is the code, maybe it can be helpful to others.

// HACK: fix excel empty columns/rows disappears
let _excelRowR = null
const ACharCode = 'A'.charCodeAt(0)
function columnLettersToNumber (letters) {
  let n = 0
  letters.split('').forEach(letter => {
    const v = letter.charCodeAt(0) - ACharCode + 1
    n = n * 26 + v
  })
  return n
}
function columnNumberToLetters (n) {
  let letters = ''
  do {
    n = n - 1
    const v = n % 26
    letters = String.fromCharCode(ACharCode + v) + letters
    n = (n - v) / 26
  } while (n > 0)
  return letters
}
preprocessor.removeRowCounterInWorksheet = function (xml) {
  if (typeof (xml) !== 'string') {
    return xml
  }

  if (!_excelRowR) {
    const prefix = '_t_' + new Date().getTime() + '_' + Math.round(Math.random() * 100000) + '_'

    _excelRowR = {
      prefix: prefix,
      bothRegexp: new RegExp(`<(?:c|row)[^>]*\\s(${prefix}="([rc])([0-9]+)_([0-9]*)_([0-9]+)")[^>]*>`, 'g')
    }
  }

  let lastRowIndex = 0
  let lastColIndex = 0
  let lastRowKey = null
  let lastColKey = null
  const spaceRight = {}
  let hasSpaceRight = false

  let content = xml.replace(/<(?:c|row)[^>]*\s(r="([A-Z]*)([0-9]+)")[^>]*>/g, function (m, rowValueString, columnValue, rowValue) {
    let what
    let value
    rowValue = parseInt(rowValue, 10)
    if (!columnValue) {
      what = 'r'
      value = rowValue - lastRowIndex// delta
      lastRowIndex = rowValue
      lastColIndex = 0
      if (lastRowKey) {
        spaceRight[lastRowKey] = value
        hasSpaceRight = true
      }
      lastRowKey = 'r' + rowValue
    } else {
      columnValue = columnLettersToNumber(columnValue)
      what = 'c'
      value = columnValue - lastColIndex
      lastColIndex = columnValue
      if (lastColKey) {
        spaceRight[lastColKey] = value
        hasSpaceRight = true
      }
      lastColKey = 'c' + columnValue
    }
    return m.replace(rowValueString, `${_excelRowR.prefix}="${what}${value}_${columnValue}_${rowValue}"`)
  }).replace(/<(?:c|row)[^>]*(spans="\S+")[^>]*>/g, function (m, rowValue) {
    return m.replace(rowValue, '')
  })

  if (hasSpaceRight) {
    content = _excelRowR.prefix + '_spaceRight=' + JSON.stringify(spaceRight) + '#' + content
  }
  return content
}
const _buildXML = builder.buildXML
builder.buildXML = function (xml, data, options, callback) {
  return _buildXML.call(this, xml, data, options, (err, xmlResult) => {
    if (_excelRowR && typeof xmlResult === 'string') {
      let spaceRight = {}
      if (xmlResult.startsWith(_excelRowR.prefix + '_spaceRight=')) {
        const start = xmlResult.indexOf('=')
        const end = xmlResult.indexOf('#')
        const objString = xmlResult.substring(start + 1, end)
        spaceRight = JSON.parse(objString)
        xmlResult = xmlResult.substring(end + 1)
      }

      let realRowIndex = 0
      let realColIndex = 0
      let lastRow = 0
      let lastCol = 0
      let foundRows = false
      const oldToNewCellsMap = {}
      xmlResult = xmlResult.replace(_excelRowR.bothRegexp, function (m, rowValueString, what, delta, columnValue, rowValue) {
        foundRows = true
        delta = parseInt(delta, 10)
        // console.log(rowValueString, what, delta, columnValue, rowValue)

        if (what === 'r') {
          rowValue = parseInt(rowValue, 10)
          if (lastRow >= rowValue) {
            const s = spaceRight['r' + lastRow]
            if (s) {
              delta = s
            }
          }
          realRowIndex += delta
          realColIndex = 0
          lastCol = 0
          lastRow = rowValue
          return m.replace(rowValueString, `r="${realRowIndex}"`)
        } else if (what === 'c') {
          rowValue = parseInt(rowValue, 10)
          columnValue = parseInt(columnValue, 10)
          if (lastCol >= columnValue) {
            const s = spaceRight['c' + lastCol]
            if (s) {
              delta = s
            }
          }
          realColIndex += delta
          lastCol = columnValue
          const oldCellName = `${columnNumberToLetters(columnValue)}${rowValue}`
          const newCellName = `${columnNumberToLetters(realColIndex)}${realRowIndex}`
          oldToNewCellsMap[oldCellName] = oldToNewCellsMap[oldCellName] || []
          oldToNewCellsMap[oldCellName].push(newCellName)
          return m.replace(rowValueString, `r="${newCellName}"`)
        }

        return m
      })

      if (foundRows) {
        // check merge
        xmlResult = xmlResult.replace(/<mergeCells[^>]*>(.*)<\/mergeCells>/g, function (m, content) {
          const matches = [...content.matchAll(/<mergeCell[^>]*\sref="([A-Z0-9]+):([A-Z0-9]+)"[^>]*>/g)]
          const newContent = []
          matches.forEach(match => {
            const cell1 = match[1]
            const cell2 = match[2]
            const newCells1 = oldToNewCellsMap[cell1]
            const newCells2 = oldToNewCellsMap[cell2]
            if (newCells1 && newCells2) {
              if (newCells1.length === newCells2.length) {
                for (let index = 0; index < newCells1.length; index++) {
                  const newCell1 = newCells1[index]
                  const newCell2 = newCells2[index]
                  // console.log(cell1, cell2, newCell1, newCell2)
                  newContent.push(`<mergeCell ref="${newCell1}:${newCell2}"/>`)
                }
              } else {
                console.log(cell1, cell2, newCells1, newCells2)
              }
            }
          })
          return `<mergeCells count="${newContent.length}">${newContent.join('')}</mergeCells>`
        })
      }
    }
    return callback(err, xmlResult)
  })
}

UPDATE: code changed to fix the merged cells problem in #90

@Alexanderdunlop
Copy link

Alexanderdunlop commented Mar 10, 2021

Just in case anyone wants a work around until this bug is fixed.
If you paint the Excel rows.

image

The cells will keep their position.
This is something we are doing and works a charm!

image

@ghost
Copy link

ghost commented Aug 1, 2024

I am currently working around this issue using aliases (which one doesn't matter - just don't output anything):
image

That's a bit more suitable to us than the coloring, because that will be visible in the final report as well, while using the alias expression will just result into an empty cell - but enforced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P: High S: on hold The issue is on hold until further notice T: bug Functionality that does not work as intended/expected
Projects
None yet
Development

No branches or pull requests

4 participants