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

Feature/build tree #1

Merged
merged 9 commits into from
Apr 15, 2024
Merged

Feature/build tree #1

merged 9 commits into from
Apr 15, 2024

Conversation

rgtj2
Copy link
Contributor

@rgtj2 rgtj2 commented Apr 11, 2024

Add build_tree and has_children? to NestedLines

rgtj2 added 3 commits April 11, 2024 14:32
Add functionality for building a tree representation of the nested line nubmers.
Add method for .has_children?
@rgtj2 rgtj2 requested review from jhixson and deepankar-j April 11, 2024 19:45
jhixson
jhixson previously approved these changes Apr 11, 2024
Copy link
Collaborator

@jhixson jhixson left a comment

Choose a reason for hiding this comment

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

When I have a line representation like this:

%NestedLines{
  lines: [
    [1],
    [0, 1],
    [0, 1],
    [0, 1],
    [0, 1],
    [0, 1],
    [1],
    [0, 1],
    [0, 1],
    [0, 1],
    [0, 1],
    [0, 1],
    [0, 0, 1]
  ]
}

The build_tree output gives me

[
  %{
    line: "1",
    children: [
      %{line: "1.1", children: []},
      %{line: "1.2", children: []},
      %{line: "1.3", children: []},
      %{line: "1.4", children: []},
      %{line: "1.5", children: []}
    ]
  },
  %{
    line: "2",
    children: [
      %{line: "2.1", children: [%{line: "2.5.1", children: []}]},
      %{line: "2.2", children: []},
      %{line: "2.3", children: []},
      %{line: "2.4", children: []},
      %{line: "2.5", children: []}
    ]
  }
]

Should that child line be under this map instead? %{line: "2.5", children: []}

@jhixson jhixson dismissed their stale review April 11, 2024 20:28

Found potential issue

@jhixson jhixson self-requested a review April 11, 2024 20:31
Update logic for adding child line to siblings:
Push onto the front of the list instead of appending to the end.
Check the proper line property on the latest tree line when looking for the parent line.
Update build_tree to reverse all children, needed with change to sort of child lines.
Add tests.
@rgtj2
Copy link
Contributor Author

rgtj2 commented Apr 11, 2024

Adjusted logic for adding lines to the proper parent and added more tests.

rgtj2 and others added 5 commits April 11, 2024 16:46
Use indentation for the first line
Update usage documentation to include a blurb about build_tree.
Update documentation to use line numbers instead of internal lines representation
Change .build_tree to .tree
@deepankar-j
Copy link
Contributor

Looks awesome! I like that the tree representation is explicit 👍.

🚀

Copy link
Contributor

@deepankar-j deepankar-j left a comment

Choose a reason for hiding this comment

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

Ship it! 🚀 🌕

@deepankar-j deepankar-j merged commit db45209 into main Apr 15, 2024
2 checks passed
@rgtj2 rgtj2 deleted the feature/build-tree branch April 15, 2024 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants