Make signatures of `simpleCellAddressToString` and `simpleCellRangeToString` more logical
Description
Currently sheetId needs to be passed twice to simpleCellAddressToString:
const cellAddress = { sheet: sheetId, row: 0, col: 3};
const cellAddressAsString = hfInstance.simpleCellAddressToString(cellAddress, sheetId);
~~The second argument to simpleCellAddressToString should be optional. Do not remove it completely to keep backwards compatibility.~~ See comments
Links
For a reason. Those are not the same. See #22 for a discussion on this topic.
You can find examples in tests:
expect(simpleCellAddressToString(sheetIndexMappingFunction, adr('A1'), 0)).toEqual('A1')
expect(simpleCellAddressToString(sheetIndexMappingFunction, adr('A1'), 1)).toEqual('Sheet1!A1')
IMO making it optional is possible but will be harmful. It only solidifies misunderstanding of references. But if you decide to make it optional anyway, please keep in mind that it is a functionality, should not be interpreted as backward compatibility or removed in the future releases.
@wojciechczerniak thank you for the clarification. I think I got it.
The second argument of simpleCellAddressToString is a sheet context. The sheet name is included in the resulting string if and only if the context is different than the cellAddress.sheet.
In that case: First of all, the documentation is wrong:
@param {number} sheetId - context used in case of missing sheet in the first argument
Second, I'd rather change the signature of this function to something like this (but that would be a breaking change):
public simpleCellAddressToString(cellAddress: SimpleCellAddress, options: { includeSheetName?: boolean } = {}): string | undefined {
Third, the analogous changes should be applied to simpleCellRangeToString
Let's change the method signature but keep the old version still working. The second parameter should be { includeSheetName?: boolean } | number