Add basic support for callout blocks #29
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently, it just prints the content of the block but ignores the emoji and color. Closes #26
Previously, the Block class constructor would iterate through all of the top-level keys of the Notion block objects, dynamically adding them all as attributes the the Block instance. Then, it would loop through all of the block-specific keys and also add them as attributes. Although this was working, in some cases the block-specific keys would over-ride the top-level keys (e.g., the "type" key in particular was being shadowed in a few cases). Dynamically adding the keys makes it more difficult to read and debug the code, since you don't know what keys are going to be set until run-time. This makes writing plugins more difficult. Also, errors due to changes in the notion API (or even issues with poorly constructed mock test data) don't appear until the expected keys are used in the `to_pandoc` calls, rather than during the constructors. This commit explicilty sets all of the top-level keys. It also moves the "type" key into the "notion_type" attribute, which avoids shadowing issues if the block-specific object has a "type" key. The "id" key was also moved to "notion_id". After making this change, the unit tests were failing since now some of the top-level keys that previously weren't being set in the unit tests were now missing. To ensure that these keys are always present, the tests were updated to use a set of mock objects that ensure the keys always exist. Furthermore, the tests related to the rich_text objects were split out into their own file. Duplication in the test was also removed through the addition of the `process_block` and `process_parent_block` functions. There is a second refactor which I believe should occur, which is to move the block-specific attributes into each block's constructors. This will make the block-specific attribute construction more explicit as well, which is good for all of the reasons stated above. A bit of work in this direction was made, but more work is needed.
Although it makes a few of the names kind of long, the consistency seems worthwhile.
The json file isn't needed or worth maintaining now that we have end to end tests.
… classes See the deleted TODO comments in the commit for the motiviation for these refactors.
xaker00
approved these changes
May 17, 2022
n2y/main.py
Outdated
| @@ -12,6 +12,13 @@ | |||
| logger = None | |||
|
|
|||
|
|
|||
| def cli_main(): | |||
| print(sys.argv) | |||
There was a problem hiding this comment.
leftover print statement?
- Remove a leftover print statement - Fix argument bug - Fix issue with block retrieval that (I think) was due to the notion API changing out from under us.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@xaker00 this pull requests added support for callout blocks and (sorry) also makes several large refactors to the tests and to the block class. I suggest reviewing the commits one-by-one, since some of the refactors and the motivation for them are described in TODO comments that are added in previous commits and then removed in the subsequent commits.