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

Server: fix sheet-scoped names in case the scope and refers_to are on different sheets #2280

Merged
merged 2 commits into from Jun 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions DEVELOPER_GUIDE.md
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
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
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
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