From 163c1554c16fabf5465fec02e59f70005cae83eb Mon Sep 17 00:00:00 2001 From: Eun-chan Cho Date: Tue, 26 Dec 2023 17:54:25 +0900 Subject: [PATCH] fix: fix numbered list format according to commonmark format (#58) * test: fix numbered list * fix: numbered list requires local state * fix: change numbered list format according to commonmark from this change the output of numbered list will be: ``` 1. list1 1. list2 ``` even if the number list is sequenced. --------- Co-authored-by: Geoff Cox --- CONTRIBUTION.md | 8 +- notion2md/convertor/block.py | 53 ++-- tests/test_numbered_list.py | 457 +++++++++++++++++++++++++++++++++++ 3 files changed, 488 insertions(+), 30 deletions(-) create mode 100644 tests/test_numbered_list.py diff --git a/CONTRIBUTION.md b/CONTRIBUTION.md index a700be5..bcf3dee 100644 --- a/CONTRIBUTION.md +++ b/CONTRIBUTION.md @@ -24,4 +24,10 @@ Pull requests are welcome. 2. make changes and push to your repo 3. send pull request from your **develop** branch to this develop branch -**This is only way to give pull request to this repo. Thank you** \ No newline at end of file +**This is only way to give pull request to this repo. Thank you** + +Please make sure that you do the following before submitting your code: +1. Run the tests: `poetry run python -m unittest discover tests` +2. Format the code `poetry run black .` +3. Use isort the code `poetry run isort .` +4. Lint the code `poetry run flake8 .` \ No newline at end of file diff --git a/notion2md/convertor/block.py b/notion2md/convertor/block.py index f3cff96..7dc481e 100644 --- a/notion2md/convertor/block.py +++ b/notion2md/convertor/block.py @@ -2,8 +2,6 @@ import hashlib import os import urllib.request as request -import uuid - from urllib.parse import urlparse from cleo.io.io import IO @@ -13,7 +11,6 @@ from notion2md.console.formatter import status from notion2md.console.formatter import success from notion2md.notion_api import NotionClient - from .richtext import richtext_convertor @@ -22,8 +19,6 @@ def __init__(self, config: Config, client: NotionClient, io: IO = None): self._config = config self._client = client self._io = io - self._continued_numbered_list = False - self._numbered_list_number = 1 def convert(self, blocks: dict) -> str: outcome_blocks: str = "" @@ -32,33 +27,24 @@ def convert(self, blocks: dict) -> str: outcome_blocks = "".join([result for result in results]) return outcome_blocks - def convert_block(self, block: dict, depth=0) -> str: + def convert_block( + self, + block: dict, + depth=0, + ): outcome_block: str = "" block_type = block["type"] - # Handle the case where the block is a list item - if block_type == "numbered_list_item": - if self._continued_numbered_list: - self._numbered_list_number += 1 - else: - self._continued_numbered_list = True - self._numbered_list_number = 1 - else: - self._continued_numbered_list = False # Special Case: Block is blank - if ( - block_type == "paragraph" - and not block["has_children"] - and not block[block_type]["rich_text"] - ): + if check_block_is_blank(block, block_type): return blank() + "\n\n" # Normal Case try: if block_type in BLOCK_TYPES: outcome_block = ( - BLOCK_TYPES[block_type]( - self.collect_info(block[block_type]) - ) - + "\n\n" + BLOCK_TYPES[block_type]( + self.collect_info(block[block_type]) + ) + + "\n\n" ) else: outcome_block = f"[//]: # ({block_type} is not supported)\n\n" @@ -78,9 +64,11 @@ def convert_block(self, block: dict, depth=0) -> str: depth += 1 child_blocks = self._client.get_children(block["id"]) for block in child_blocks: - outcome_block += "\t" * depth + self.convert_block( - block, depth + converted_block = self.convert_block( + block, + depth, ) + outcome_block += "\t" * depth + converted_block except Exception as e: if self._io: self._io.write_line( @@ -102,7 +90,7 @@ def create_table(self, cell_blocks: dict): if index == 0: table = " | " + " | ".join(value) + " | " + "\n" table += ( - " | " + " | ".join(["----"] * len(value)) + " | " + "\n" + " | " + " | ".join(["----"] * len(value)) + " | " + "\n" ) continue table += " | " + " | ".join(value) + " | " + "\n" @@ -139,7 +127,6 @@ def collect_info(self, payload: dict) -> dict: # table cells if "cells" in payload: info["cells"] = payload["cells"] - info["number"] = self._numbered_list_number return info def download_file(self, url: str) -> str: @@ -182,6 +169,14 @@ def to_string(self, blocks: dict) -> str: return self.convert(blocks) +def check_block_is_blank(block, block_type): + return ( + block_type == "paragraph" + and not block["has_children"] + and not block[block_type]["rich_text"] + ) + + # Converting Methods def paragraph(info: dict) -> str: return info["text"] @@ -217,7 +212,7 @@ def numbered_list_item(info: dict) -> str: """ input: item:dict = {"number":int, "text":str} """ - return f"{info['number']}. {info['text']}" + return f"1. {info['text']}" def to_do(info: dict) -> str: diff --git a/tests/test_numbered_list.py b/tests/test_numbered_list.py new file mode 100644 index 0000000..1d3541f --- /dev/null +++ b/tests/test_numbered_list.py @@ -0,0 +1,457 @@ +import unittest +from unittest.mock import MagicMock +from unittest.mock import patch + +from notion2md.exporter.block import StringExporter # type: ignore + +expected_md = """1. Thing1: + +\t1. Thing2: + +\t\t1. one + +\t\t1. two + +
+ +Stuff + +
+ +1. Schema + +\t1. id (integer) + +\t1. type (string) + +""" + +mock_responses = [ + { + "object": "list", + "results": [ + { + "object": "block", + "id": "94641568-819d-4a12-a313-c64a3d176cf5", + "parent": { + "type": "page_id", + "page_id": "811969e6-f4f5-4e27-a6ac-2b58c2f22e26", + }, + "created_time": "2023-09-02T22:46:00.000Z", + "last_edited_time": "2023-09-02T22:48:00.000Z", + "created_by": { + "object": "user", + "id": "a7c39264-1886-4b50-ba48-87d1791cf3f4", + }, + "last_edited_by": { + "object": "user", + "id": "a7c39264-1886-4b50-ba48-87d1791cf3f4", + }, + "has_children": True, + "archived": False, + "type": "numbered_list_item", + "numbered_list_item": { + "rich_text": [ + { + "type": "text", + "text": {"content": "Thing1:", "link": None}, + "annotations": { + "bold": False, + "italic": False, + "strikethrough": False, + "underline": False, + "code": False, + "color": "default", + }, + "plain_text": "Thing1:", + "href": None, + } + ], + "color": "default", + }, + }, + { + "object": "block", + "id": "8138f3ff-8fb8-4fb6-8692-e69f7b2a64a5", + "parent": { + "type": "page_id", + "page_id": "811969e6-f4f5-4e27-a6ac-2b58c2f22e26", + }, + "created_time": "2023-09-02T22:48:00.000Z", + "last_edited_time": "2023-09-02T22:48:00.000Z", + "created_by": { + "object": "user", + "id": "a7c39264-1886-4b50-ba48-87d1791cf3f4", + }, + "last_edited_by": { + "object": "user", + "id": "a7c39264-1886-4b50-ba48-87d1791cf3f4", + }, + "has_children": False, + "archived": False, + "type": "paragraph", + "paragraph": {"rich_text": [], "color": "default"}, + }, + { + "object": "block", + "id": "36e6d74c-0069-4089-a0db-3979467a15f6", + "parent": { + "type": "page_id", + "page_id": "811969e6-f4f5-4e27-a6ac-2b58c2f22e26", + }, + "created_time": "2023-09-02T22:48:00.000Z", + "last_edited_time": "2023-09-02T22:48:00.000Z", + "created_by": { + "object": "user", + "id": "a7c39264-1886-4b50-ba48-87d1791cf3f4", + }, + "last_edited_by": { + "object": "user", + "id": "a7c39264-1886-4b50-ba48-87d1791cf3f4", + }, + "has_children": False, + "archived": False, + "type": "paragraph", + "paragraph": { + "rich_text": [ + { + "type": "text", + "text": {"content": "Stuff", "link": None}, + "annotations": { + "bold": False, + "italic": False, + "strikethrough": False, + "underline": False, + "code": False, + "color": "default", + }, + "plain_text": "Stuff", + "href": None, + } + ], + "color": "default", + }, + }, + { + "object": "block", + "id": "994496db-727e-42b0-bf7c-a346ad5070b8", + "parent": { + "type": "page_id", + "page_id": "811969e6-f4f5-4e27-a6ac-2b58c2f22e26", + }, + "created_time": "2023-09-02T22:46:00.000Z", + "last_edited_time": "2023-09-02T22:48:00.000Z", + "created_by": { + "object": "user", + "id": "a7c39264-1886-4b50-ba48-87d1791cf3f4", + }, + "last_edited_by": { + "object": "user", + "id": "a7c39264-1886-4b50-ba48-87d1791cf3f4", + }, + "has_children": False, + "archived": False, + "type": "paragraph", + "paragraph": {"rich_text": [], "color": "default"}, + }, + { + "object": "block", + "id": "e6bb8ac2-7af3-4cbe-af41-e7ac75fe6231", + "parent": { + "type": "page_id", + "page_id": "811969e6-f4f5-4e27-a6ac-2b58c2f22e26", + }, + "created_time": "2023-09-02T22:46:00.000Z", + "last_edited_time": "2023-09-02T22:48:00.000Z", + "created_by": { + "object": "user", + "id": "a7c39264-1886-4b50-ba48-87d1791cf3f4", + }, + "last_edited_by": { + "object": "user", + "id": "a7c39264-1886-4b50-ba48-87d1791cf3f4", + }, + "has_children": True, + "archived": False, + "type": "numbered_list_item", + "numbered_list_item": { + "rich_text": [ + { + "type": "text", + "text": {"content": "Schema", "link": None}, + "annotations": { + "bold": False, + "italic": False, + "strikethrough": False, + "underline": False, + "code": False, + "color": "default", + }, + "plain_text": "Schema", + "href": None, + } + ], + "color": "default", + }, + }, + ], + "next_cursor": None, + "has_more": False, + "type": "block", + "block": {}, + }, + { + "object": "list", + "results": [ + { + "object": "block", + "id": "a86e3565-da49-47ba-9e96-2c22b426f66b", + "parent": { + "type": "block_id", + "block_id": "94641568-819d-4a12-a313-c64a3d176cf5", + }, + "created_time": "2023-09-02T22:46:00.000Z", + "last_edited_time": "2023-09-02T22:48:00.000Z", + "created_by": { + "object": "user", + "id": "a7c39264-1886-4b50-ba48-87d1791cf3f4", + }, + "last_edited_by": { + "object": "user", + "id": "a7c39264-1886-4b50-ba48-87d1791cf3f4", + }, + "has_children": True, + "archived": False, + "type": "numbered_list_item", + "numbered_list_item": { + "rich_text": [ + { + "type": "text", + "text": {"content": "Thing2:", "link": None}, + "annotations": { + "bold": False, + "italic": False, + "strikethrough": False, + "underline": False, + "code": False, + "color": "default", + }, + "plain_text": "Thing2:", + "href": None, + } + ], + "color": "default", + }, + } + ], + "next_cursor": None, + "has_more": False, + "type": "block", + "block": {}, + }, + { + "object": "list", + "results": [ + { + "object": "block", + "id": "da029b1b-f4ae-40d5-95dd-c0a869f8196d", + "parent": { + "type": "block_id", + "block_id": "a86e3565-da49-47ba-9e96-2c22b426f66b", + }, + "created_time": "2023-09-02T22:46:00.000Z", + "last_edited_time": "2023-09-02T22:46:00.000Z", + "created_by": { + "object": "user", + "id": "a7c39264-1886-4b50-ba48-87d1791cf3f4", + }, + "last_edited_by": { + "object": "user", + "id": "a7c39264-1886-4b50-ba48-87d1791cf3f4", + }, + "has_children": False, + "archived": False, + "type": "numbered_list_item", + "numbered_list_item": { + "rich_text": [ + { + "type": "text", + "text": {"content": "one", "link": None}, + "annotations": { + "bold": False, + "italic": False, + "strikethrough": False, + "underline": False, + "code": False, + "color": "default", + }, + "plain_text": "one", + "href": None, + } + ], + "color": "default", + }, + }, + { + "object": "block", + "id": "1a490359-e05a-4b3e-b39e-0f2eb867b527", + "parent": { + "type": "block_id", + "block_id": "a86e3565-da49-47ba-9e96-2c22b426f66b", + }, + "created_time": "2023-09-02T22:46:00.000Z", + "last_edited_time": "2023-09-02T22:46:00.000Z", + "created_by": { + "object": "user", + "id": "a7c39264-1886-4b50-ba48-87d1791cf3f4", + }, + "last_edited_by": { + "object": "user", + "id": "a7c39264-1886-4b50-ba48-87d1791cf3f4", + }, + "has_children": False, + "archived": False, + "type": "numbered_list_item", + "numbered_list_item": { + "rich_text": [ + { + "type": "text", + "text": {"content": "two", "link": None}, + "annotations": { + "bold": False, + "italic": False, + "strikethrough": False, + "underline": False, + "code": False, + "color": "default", + }, + "plain_text": "two", + "href": None, + } + ], + "color": "default", + }, + }, + ], + "next_cursor": None, + "has_more": False, + "type": "block", + "block": {}, + }, + { + "object": "list", + "results": [ + { + "object": "block", + "id": "612fac7c-d5a9-4946-819d-efd14b444760", + "parent": { + "type": "block_id", + "block_id": "e6bb8ac2-7af3-4cbe-af41-e7ac75fe6231", + }, + "created_time": "2023-09-02T22:46:00.000Z", + "last_edited_time": "2023-09-02T22:46:00.000Z", + "created_by": { + "object": "user", + "id": "a7c39264-1886-4b50-ba48-87d1791cf3f4", + }, + "last_edited_by": { + "object": "user", + "id": "a7c39264-1886-4b50-ba48-87d1791cf3f4", + }, + "has_children": False, + "archived": False, + "type": "numbered_list_item", + "numbered_list_item": { + "rich_text": [ + { + "type": "text", + "text": {"content": "id (integer)", "link": None}, + "annotations": { + "bold": False, + "italic": False, + "strikethrough": False, + "underline": False, + "code": False, + "color": "default", + }, + "plain_text": "id (integer)", + "href": None, + } + ], + "color": "default", + }, + }, + { + "object": "block", + "id": "29a6591f-67cd-4d34-9ab5-2ac917bdbcab", + "parent": { + "type": "block_id", + "block_id": "e6bb8ac2-7af3-4cbe-af41-e7ac75fe6231", + }, + "created_time": "2023-09-02T22:46:00.000Z", + "last_edited_time": "2023-09-02T22:49:00.000Z", + "created_by": { + "object": "user", + "id": "a7c39264-1886-4b50-ba48-87d1791cf3f4", + }, + "last_edited_by": { + "object": "user", + "id": "a7c39264-1886-4b50-ba48-87d1791cf3f4", + }, + "has_children": False, + "archived": False, + "type": "numbered_list_item", + "numbered_list_item": { + "rich_text": [ + { + "type": "text", + "text": {"content": "type (string)", "link": None}, + "annotations": { + "bold": False, + "italic": False, + "strikethrough": False, + "underline": False, + "code": False, + "color": "default", + }, + "plain_text": "type (string)", + "href": None, + } + ], + "color": "default", + }, + }, + ], + "next_cursor": None, + "has_more": False, + "type": "block", + "block": {}, + }, +] + + +class NumberedListTest(unittest.TestCase): + @patch("os.environ") + def test_get_children(self, mock_env) -> None: + # Mock environment variable + mock_env["NOTION_TOKEN"] = "mock_token" + + # Mock the Client class + with patch("notion2md.notion_api.Client") as MockedClient: + # Mock the blocks.children.list method to return different values on subsequent calls + mock_blocks_children_list = MagicMock() + mock_blocks_children_list.side_effect = mock_responses + + # Attach the mock to the Client instance + MockedClient.return_value.blocks.children.list = ( + mock_blocks_children_list + ) + + block_id = "1" + md: str = StringExporter(block_id).export() + + assert md == expected_md + + +if __name__ == "__main__": + unittest.main()