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

Custom printer support #319

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

technicalpickles
Copy link

I'd like to be able to use a custom printer from the command-line. Specifically, ruby-prof-speedscope.

I've extended the CLI to:

  • update --printer to allow a class name
  • add --printer-require for the file to require to use a printer

I ended up adding facets to get the String#modularize method. If this approach is generally acceptable, but an extra dependency isn't, I'm happy to extract copy it or ActiveSupport's implementation, similar to what is done for constantize

This allows the use of gems like https://github.com/chanzuckerberg/ruby-prof-speedscope :

- change `--printer` can take a class name instead of just a name of a builtin
- add `--printer-require` for a require for a printer
- add `load_printer` which is called during parse_args... after they are
  parsed, but before checking the output directory

I initially tried not adding --printer-require, and re-using
--require-noprof, but that didn't load the printer in time.
@cfis
Copy link
Member

cfis commented Apr 17, 2023

Did you still want to merge this? I saw the original github issue was closed so I figured this was no longer valid, but looking at it maybe it is?

I'm good with this, but as you suspected, without the facets dependency. If you update the PR without it, I am happy to merge it.

Sorry for not looking at this sooner.

@technicalpickles
Copy link
Author

I think it's still useful. The original PR was accidentally opened against my fork, not this 🙃

I'll update and ping when ready.

@cfis
Copy link
Member

cfis commented Apr 17, 2023

Great thanks.

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.

None yet

2 participants