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

Creates an optional callback to check for other paths to omit #33

Merged
merged 12 commits into from Mar 21, 2023

Conversation

broguinn
Copy link
Contributor

@broguinn broguinn commented Mar 16, 2023

Love this repo - I think it'd be great if we could omit a few more paths as-needed.

This PR:

  • Adds an option for pathFilterCallback which can be used to filter out extra lines if the callback returns false for the line's path.
  • Adds a test for this option.

I'll add to the readme for this option next. Let me know if you have any questions!

@sindresorhus
Copy link
Owner

Just out of curiosity, what kind of stack trace lines would you use it for?

@sindresorhus
Copy link
Owner

It should be a function instead that receives a string with the stack line and is expected to return a boolean weather to keep it. That's more flexible.

@broguinn
Copy link
Contributor Author

@sindresorhus our team is writing a proxy for a class of objects in a view tree. This is great for adding metadata to our logging and terrible for readability for our call stack. We want to filter out all calls to the proxy.

Happy to make this a callback - give me a few minutes.

@broguinn broguinn changed the title Creates an optional regexp to check for other paths to omit Creates an optional callback to check for other paths to omit Mar 16, 2023
index.d.ts Outdated
*/
readonly extraPathRegex?: RegExp;
readonly pathFilterCallback?: (string) => boolean;
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
readonly pathFilterCallback?: (string) => boolean;
readonly pathFilter?: (string) => boolean;

I don't think Callback adds anything. The method signature is clear.

index.d.ts Outdated
@@ -18,9 +18,13 @@ export type Options = {
readonly basePath?: string;

/**
Remove any paths that match this regexp from the stack.
Remove any paths that return false from this callback.
Copy link
Owner

Choose a reason for hiding this comment

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

We are not just removing the paths, we are removing the stack lines with the matching paths. This should be clearer.

index.d.ts Outdated

Example with `path => /unicorn/.test(path)` as `pathFilterCallback`:

`/Users/sindresorhus/dev/clean-stack/unicorn.js:2:15` → ``
Copy link
Owner

Choose a reason for hiding this comment

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

I think a full example would be useful in this case. Something like:

@example

import cleanStack from 'clean-stack';

const error = new Error('Missing unicorn');

console.log(cleanStack(error.stack));
// Error: Missing unicorn
//     at Object.<anonymous> (/Users/sindresorhus/dev/clean-stack/unicorn.js:2:15)
//     at Object.<anonymous> (/Users/sindresorhus/dev/clean-stack/unicorn.js:2:15)

console.log(cleanStack(error.stack, {pathFilter ...}));
// Error: Missing unicorn
//     at Object.<anonymous> (/Users/sindresorhus/dev/clean-stack/unicorn.js:2:15)

readme.md Outdated
// at Object.<anonymous> (/Users/sindresorhus/dev/clean-stack/unicorn.js:2:15)
// at Object.<anonymous> (/Users/sindresorhus/dev/clean-stack/omit-me.js:1:16)

const pathFilter = path => /omit-me/.test(path);
Copy link
Owner

Choose a reason for hiding this comment

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

This is the inverse of how filter generally works. Filter keeps the element on true and discards on false.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is even how I implemented the pathFilter - I just missed the boolean inversion. Thanks.

@sindresorhus sindresorhus merged commit fa2b9c3 into sindresorhus:main Mar 21, 2023
3 checks passed
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