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

Fixed bug with making list at the end of the document #260

Merged
merged 2 commits into from
Apr 27, 2022

Conversation

lagartoverde
Copy link
Contributor

@lagartoverde lagartoverde added the bug Something isn't working label Apr 22, 2022
@lagartoverde lagartoverde requested a review from abeforgit April 22, 2022 08:10
@lagartoverde lagartoverde self-assigned this Apr 22, 2022
Copy link
Member

@abeforgit abeforgit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Breaks a lot of treewalker tests, please check

Copy link
Member

@nvdk nvdk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addresses the bug and there are no longer any failing tests. I don't fully grasp why we would not return the starting node though, so I'd appreciate a bit more info :)

@abeforgit
Copy link
Member

The walkFromNode function really walks (away) from the node. Aka there is an implicit assumption that the node we are walking from has already been yielded in the generator in a previous step. Therefore, returning the node we are walking from should actually always result in an infinite loop in the generator, and can never be the correct return value.

@abeforgit abeforgit merged commit e254c0f into development Apr 27, 2022
@abeforgit abeforgit deleted the bugfix/making-list-end-document branch April 27, 2022 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants