Skip to content

Repo#ls_tree - #67

Open
dometto wants to merge 2 commits into
gollum:masterfrom
dometto:improve_lstree
Open

Repo#ls_tree#67
dometto wants to merge 2 commits into
gollum:masterfrom
dometto:improve_lstree

Conversation

@dometto

@dometto dometto commented Jan 11, 2025

Copy link
Copy Markdown
Member
  • add support for non-recursive lstree
  • add support for lstree for specific path

  * add support for non-recursive lstree
  * add support for lstree for specific path
@dometto dometto changed the title Repo#ls_tree: Repo#ls_tree Jan 11, 2025

@bartkamphorst bartkamphorst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Review of the first set of comments, please see if they are still useful.

Comment thread lib/rugged_adapter/git_layer_rugged.rb Outdated
results = []
@repo.lookup(sha).tree.walk(:postorder) do |root, entry|
results << ::Gollum::Git::Tree.tree_entry_from_rugged_hash(entry, root)
def lstree(sha, path, options = {})

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In RJGit adapter, the path argument in lstree has a default of nil. Should we do the same here?

results << ::Gollum::Git::Tree.tree_entry_from_rugged_hash(entry, root)
def lstree(sha, path, options = {})

if path

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What about:

tree = @repo.lookup(sha).tree
tree = tree / path if path

Instead of the if/else?

end
results
else
tree.map do |entry|

@bartkamphorst bartkamphorst Jan 11, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

tree.map { |entry| Gollum::Git::Tree.tree_entry_from_rugged_hash(entry, root) }

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.

2 participants