Skip to content

Commit

Permalink
Server: fix sheet-scoped names in case the scope and refers_to are on…
Browse files Browse the repository at this point in the history
… different sheets (#2280)

* improve testing by introducing a space in sheet name

* server: fixing sheet scoped names and tests
  • Loading branch information
fzumstein committed Jun 8, 2023
1 parent 4c3d5c8 commit 7d1a4bb
Show file tree
Hide file tree
Showing 7 changed files with 86 additions and 41 deletions.
4 changes: 4 additions & 0 deletions DEVELOPER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ The 3rd party Open Source licenses document is built with `cargo about generate

## Office.js add-ins

Script Lab: figuring out the exact syntax for Office.js works is easiest done in the Script Lab add-in that can be installed via Excel's add-in store.

To set up a development environment for the xlwings.js library, you need to do the following:

* Generate dev certificates (otherwise, icons and dialogs won't load and Excel on the web won't load the manifest at all): download `mkcert` from the [GH Release page](https://github.com/FiloSottile/mkcert/releases), rename the file to `mkcert`, then run the following commands:
```
cd xlwingsjs
Expand Down
Binary file modified tests/test_engines/engines.xls
Binary file not shown.
Binary file modified tests/test_engines/engines.xlsb
Binary file not shown.
Binary file modified tests/test_engines/engines.xlsm
Binary file not shown.
46 changes: 27 additions & 19 deletions tests/test_engines/test_engines.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,21 @@
"book_scope": True,
},
{
"name": "two",
"name": "two", # VBA/GS send: "'Sheet 1'!two"
"sheet_index": 0,
"address": "C7:D8",
"scope_sheet_name": "Sheet1",
"scope_sheet_name": "Sheet 1",
"scope_sheet_index": 0,
"book_scope": False,
},
{
"name": "two", # VBA/GS send: "Sheet2!two"
"sheet_index": 2,
"address": "B3",
"book_scope": False,
"scope_sheet_name": "Sheet2",
"scope_sheet_index": 1,
},
{
"name": "two",
"sheet_index": 1,
Expand All @@ -64,7 +72,7 @@
],
"sheets": [
{
"name": "Sheet1",
"name": "Sheet 1",
"values": [
["a", "b", "c", ""],
[1.1, 2.2, 3.3, "2021-01-01T00:00:00.000Z"],
Expand Down Expand Up @@ -373,9 +381,9 @@ def test_write_basic_types(book):

# sheets
def test_sheet_access(book):
assert book.sheets[0] == book.sheets["Sheet1"]
assert book.sheets[0] == book.sheets["Sheet 1"]
assert book.sheets[1] == book.sheets["Sheet2"]
assert book.sheets[0].name == "Sheet1"
assert book.sheets[0].name == "Sheet 1"
assert book.sheets[1].name == "Sheet2"


Expand All @@ -386,7 +394,7 @@ def test_sheet_active(book):

def test_sheets_iteration(book):
for ix, sheet in enumerate(book.sheets):
assert sheet.name == "Sheet1" if ix == 0 else "Sheet2"
assert sheet.name == "Sheet 1" if ix == 0 else "Sheet2"


# book name
Expand Down Expand Up @@ -472,9 +480,7 @@ def test_picture_exists(book):
# Named Ranges
def test_named_range_book_scope(book):
sheet1 = book.sheets[0]
sheet2 = book.sheets[1]
assert sheet1["one"].address == "$A$1"
assert sheet2["two"].address == "$A$1:$A$2"


def test_named_range_sheet_scope(book):
Expand Down Expand Up @@ -502,7 +508,7 @@ def test_named_range_book_change_value(book):

# Names collection
def test_names_len(book):
assert len(book.names) == 3
assert len(book.names) == 4


def test_names_index_vs_name(book):
Expand All @@ -512,22 +518,24 @@ def test_names_index_vs_name(book):

@pytest.mark.skipif(engine == "calamine", reason="doesn't support local scope yet")
def test_name_local_scope1(book):
assert book.names[1].name == "Sheet1!two"
assert book.names[1].name == "'Sheet 1'!two"
assert book.names[2].name == "Sheet2!two"


@pytest.mark.skipif(engine == "calamine", reason="doesn't support local scope yet")
def test_name_local_scope2(book):
assert book.sheets["Sheet1"].names[0].name == "Sheet1!two"
assert book.sheets["Sheet 1"].names[0].name == "'Sheet 1'!two"
assert book.sheets["Sheet2"].names[0].name == "Sheet2!two"


def test_name_refers_to(book):
assert book.names[0].refers_to == "=Sheet1!$A$1"
assert book.names[0].refers_to == "='Sheet 1'!$A$1"


def test_name_refers_to_range(book):
assert book.names[0].refers_to_range == book.sheets[0]["A1"]
assert book.names[1].refers_to_range == book.sheets[0]["C7:D8"]
assert book.names[2].refers_to_range == book.sheets[1]["A1:A2"]
assert book.names[3].refers_to_range == book.sheets[1]["A1:A2"]


def test_name_contains(book):
Expand All @@ -540,15 +548,15 @@ def test_names_iter(book):
assert name.refers_to_range == book.sheets[0]["A1"]
elif ix == 1:
assert name.refers_to_range == book.sheets[0]["C7:D8"]
elif ix == 2:
elif ix == 3:
assert name.refers_to_range == book.sheets[1]["A1:A2"]


@pytest.mark.skipif(engine != "remote", reason="requires remote engine")
@pytest.mark.skipif(engine == "calamine", reason="unsupported by calamine")
def test_range_get_name(book):
assert book.sheets[0]["A1"].name == book.names[0]
assert book.sheets[0]["C7:D8"].name == book.names[1]
assert book.sheets[1]["A1:A2"].name == book.names[2]
assert book.sheets[0]["A1"].name.name == "one"
assert book.sheets[0]["C7:D8"].name.name == "'Sheet 1'!two"
assert book.sheets[1]["A1:A2"].name.name == "two"
assert book.sheets[0]["X1"].name is None


Expand Down Expand Up @@ -581,7 +589,7 @@ def test_sheet_name_delete(book):
assert book.json()["actions"][0]["func"] == "nameDelete"
assert book.json()["actions"][0]["args"] == [
"one",
"=Sheet1!$A$1",
"='Sheet 1'!$A$1",
"one",
0,
True,
Expand Down
6 changes: 4 additions & 2 deletions xlwings/pro/_xlremote.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@

datetime_pattern = (
pattern
) = r"^(-?(?:[1-9][0-9]*)?[0-9]{4})-(1[0-2]|0[1-9])-(3[01]|0[1-9]|[12][0-9])T(2[0-3]|[01][0-9]):([0-5][0-9]):([0-5][0-9])(\.[0-9]+)?(Z|[+-](?:2[0-3]|[01][0-9]):[0-5][0-9])?$"
) = r"^(-?(?:[1-9][0-9]*)?[0-9]{4})-(1[0-2]|0[1-9])-(3[01]|0[1-9]|[12][0-9])T(2[0-3]|[01][0-9]):([0-5][0-9]):([0-5][0-9])(\.[0-9]+)?(Z|[+-](?:2[0-3]|[01][0-9]):[0-5][0-9])?$" # noqa: E501
datetime_regex = re.compile(pattern)


Expand Down Expand Up @@ -447,7 +447,9 @@ def names(self):
api = [
name
for name in self.book.api["names"]
if name["sheet_index"] + 1 == self.index and not name["book_scope"]
if name["scope_sheet_index"] is not None
and name["scope_sheet_index"] + 1 == self.index
and not name["book_scope"]
]
return Names(parent=self, api=api)

Expand Down
71 changes: 51 additions & 20 deletions xlwingsjs/devserver.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def integration_test_read(data: dict = Body):
expected_data = expected_body["Google Apps Script"]
elif data["client"] == "Microsoft Office Scripts":
expected_data = expected_body["Office Scripts"]
assert data == expected_data, "Body differs (Make sure to select cell Sheet1!A1)"
assert data == expected_data, "Body differs (Make sure to select cell 'Sheet 1'!A1)"
book.app.alert("OK", title="Integration Test Read")
return book.json()

Expand Down Expand Up @@ -259,14 +259,22 @@ async def exception_handler(request, exception):
"name": "two",
"sheet_index": 0,
"address": "C7:D8",
"scope_sheet_name": "Sheet1",
"scope_sheet_name": "Sheet 1",
"scope_sheet_index": 0,
"book_scope": False,
},
{
"name": "two",
"sheet_index": 2,
"address": "B3",
"scope_sheet_name": "Sheet2",
"scope_sheet_index": 1,
"book_scope": False,
},
],
"sheets": [
{
"name": "Sheet1",
"name": "Sheet 1",
"values": [
["a", "b", "c", ""],
[1.1, 2.2, 3.3, "2021-01-01T00:00:00.000Z"],
Expand Down Expand Up @@ -350,13 +358,21 @@ async def exception_handler(request, exception):
"scope_sheet_index": None,
},
{
"name": "Sheet1!two",
"name": "'Sheet 1'!two",
"sheet_index": 0,
"address": "C7:D8",
"book_scope": False,
"scope_sheet_name": "Sheet1",
"scope_sheet_name": "Sheet 1",
"scope_sheet_index": 0,
},
{
"name": "Sheet2!two",
"sheet_index": 2,
"address": "B3",
"book_scope": False,
"scope_sheet_name": "Sheet2",
"scope_sheet_index": 1,
},
{
"name": "two",
"sheet_index": 1,
Expand All @@ -368,8 +384,7 @@ async def exception_handler(request, exception):
],
"sheets": [
{
"name": "Sheet1",
# Differs between Windows and macOS
"name": "Sheet 1",
"pictures": [
{"name": "mypic1", "height": 10, "width": 20},
{"name": "mypic2", "height": 30, "width": 40},
Expand Down Expand Up @@ -462,14 +477,22 @@ async def exception_handler(request, exception):
"name": "two",
"sheet_index": 0,
"address": "C7:D8",
"scope_sheet_name": "Sheet1",
"scope_sheet_name": "Sheet 1",
"scope_sheet_index": 0,
"book_scope": False,
},
{
"name": "two",
"sheet_index": 2,
"address": "B3",
"scope_sheet_name": "Sheet2",
"scope_sheet_index": 1,
"book_scope": False,
},
],
"sheets": [
{
"name": "Sheet1",
"name": "Sheet 1",
"values": [
["a", "b", "c", ""],
[1.1, 2.2, 3.3, "2021-01-01T00:00:00.000Z"],
Expand Down Expand Up @@ -544,13 +567,29 @@ async def exception_handler(request, exception):
"book": {"name": "engines.xlsm", "active_sheet_index": 0, "selection": "A1"},
"names": [
{
"name": "Sheet1!two",
"name": "one",
"sheet_index": 0,
"address": "A1",
"scope_sheet_name": None,
"scope_sheet_index": None,
"book_scope": True,
},
{
"name": "'Sheet 1'!two",
"sheet_index": 0,
"address": "C7:D8",
"scope_sheet_name": "Sheet1",
"scope_sheet_name": "Sheet 1",
"scope_sheet_index": 0,
"book_scope": False,
},
{
"name": "Sheet2!two",
"sheet_index": 2,
"address": "B3",
"scope_sheet_name": "Sheet3",
"scope_sheet_index": 2,
"book_scope": False,
},
{
"name": "two",
"sheet_index": 1,
Expand All @@ -559,18 +598,10 @@ async def exception_handler(request, exception):
"scope_sheet_index": None,
"book_scope": True,
},
{
"name": "one",
"sheet_index": 0,
"address": "A1",
"scope_sheet_name": None,
"scope_sheet_index": None,
"book_scope": True,
},
],
"sheets": [
{
"name": "Sheet1",
"name": "Sheet 1",
"values": [
["a", "b", "c", ""],
[1.1, 2.2, 3.3, "2021-01-01T00:00:00.000Z"],
Expand Down

0 comments on commit 7d1a4bb

Please sign in to comment.