decode_range returns same reference for s and e when input range is a single cell #2026

2020-06-25T21:38:03Z

kerbs17 commented

2020-06-25 21:38:03 +00:00

(Migrated from github.com)

When calling decode_range with a single cell, ie: xlsx.utils.decode_range('A1'), you end up with an object where s and e are the same reference. Therefore, if you modify e.c or e.r, it will appear as if s.c or s.r has also been affected and vice versa. This code change ensures s and e are independent references so that they don't side-effect each other.

When calling `decode_range` with a single cell, ie: `xlsx.utils.decode_range('A1')`, you end up with an object where `s` and `e` are the same reference. Therefore, if you modify `e.c` or `e.r`, it will appear as if `s.c` or `s.r` has also been affected and vice versa. This code change ensures `s` and `e` are independent references so that they don't side-effect each other.

SheetJSDev (Migrated from github.com) approved these changes 2020-06-25 21:43:09 +00:00

SheetJSDev commented

2020-06-26 08:47:42 +00:00

(Migrated from github.com)

Simple test:

	it('decode_range', function() {
		var _c = "ABC", _r = "123", _C = "DEF", _R = "456";

		var r = X.utils.decode_range(_c + _r + ":" + _C + _R);
		assert(r.s != r.e);
		assert.equal(r.s.c, X.utils.decode_col(_c)); assert.equal(r.s.r, X.utils.decode_row(_r));
		assert.equal(r.e.c, X.utils.decode_col(_C)); assert.equal(r.e.r, X.utils.decode_row(_R));

		r = X.utils.decode_range(_c + _r);
		assert(r.s != r.e);
		assert.equal(r.s.c, X.utils.decode_col(_c)); assert.equal(r.s.r, X.utils.decode_row(_r));
		assert.equal(r.e.c, X.utils.decode_col(_c)); assert.equal(r.e.r, X.utils.decode_row(_r));
	});

Best place is in the API test block

Simple test: ```js it('decode_range', function() { var _c = "ABC", _r = "123", _C = "DEF", _R = "456"; var r = X.utils.decode_range(_c + _r + ":" + _C + _R); assert(r.s != r.e); assert.equal(r.s.c, X.utils.decode_col(_c)); assert.equal(r.s.r, X.utils.decode_row(_r)); assert.equal(r.e.c, X.utils.decode_col(_C)); assert.equal(r.e.r, X.utils.decode_row(_R)); r = X.utils.decode_range(_c + _r); assert(r.s != r.e); assert.equal(r.s.c, X.utils.decode_col(_c)); assert.equal(r.s.r, X.utils.decode_row(_r)); assert.equal(r.e.c, X.utils.decode_col(_c)); assert.equal(r.e.r, X.utils.decode_row(_r)); }); ``` Best place is in the `API` test block

kerbs17 commented

2020-06-26 15:58:02 +00:00

(Migrated from github.com)

Looks like tests failed to run due to a resource that has moved off bitbucket - https://bitbucket.org/openpyxl/openpyxl/src/master/

SheetJSDev commented

2020-06-26 19:14:49 +00:00

(Migrated from github.com)

As long as the new test passes, it's safe to merge. We have to update the test files repo to make it work, and at this stage it might be better to mirror the repo or just add their specimens