Skip to content

Conversation

0xferrous
Copy link
Contributor

@0xferrous 0xferrous commented Aug 11, 2025

Motivation

Currently forge selectors list shows one table of selectors per contract and that is fine. But the output is not greppable because if I wanted to see which contract had a particular selector, the contract name would be appearing before the table is printed.

Solution

--no-group option only prints one table and adds a contract field to each row which makes it greppable. And since it is a new option, the current behavior stays the same for everyone.

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

Copy link
Collaborator

@onbjerg onbjerg left a comment

Choose a reason for hiding this comment

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

Hi, thanks for opening this PR!

The current implementation seems to show duplicate selectors, i.e. if I have two contracts with a function named transfer that has the exact same signature, transfer would show up twice. Let's make sure we deduplicate selectors if --no-group is passed.

Apologies for the delay.

@onbjerg onbjerg self-assigned this Aug 25, 2025
@onbjerg onbjerg added Cmd-forge-selectors Command: forge selectors T-feature Type: feature labels Aug 25, 2025
@0xferrous
Copy link
Contributor Author

I think showing duplicates might be desirable when the intent is to grep on the output and find contracts that implement a specific method. We'd also have to decide in the deduplication case what value do we show for contract name column? Should I deduplicate the rows, but fold the contract names so that it shows something like contract_names.iter().join(", ")?

@grandizzy grandizzy moved this to In Progress in Foundry Aug 25, 2025
Copy link
Collaborator

@onbjerg onbjerg left a comment

Choose a reason for hiding this comment

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

ah I see it lists the contract as well. in that case this is ok, thanks for the pr.

@onbjerg onbjerg merged commit 33e4b4b into foundry-rs:master Aug 25, 2025
22 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Foundry Aug 25, 2025
@grandizzy grandizzy moved this from Done to Completed in Foundry Sep 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cmd-forge-selectors Command: forge selectors T-feature Type: feature
Projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

2 participants