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

fix: add codeContext property to types #43

Merged
merged 2 commits into from
May 3, 2022
Merged

fix: add codeContext property to types #43

merged 2 commits into from
May 3, 2022

Conversation

Julien-R44
Copy link
Member

Hey !
Just this little addition. I need to modify this property elsewhere after creating my Youch instance

@thetutlage
Copy link
Member

A couple of things. The codeContext doesn't feel like a right name to store "number of lines to display".

Another thing, can we just consume this property internally and only return that many number of lines? For example:

If you say new Youch({ linesToDisplay: 3 }), then the Youch JSON and HTML will only return 3 lines at the first place? We might to make modifications here https://github.com/poppinss/youch/blob/develop/src/Youch.js#L115-L118

@Julien-R44
Copy link
Member Author

Yes I totally agree. I just didn't want to create a breaking change, but if you're okay with it that's cool !

I'm not sure what you want to change here though:

youch/src/Youch.js

Lines 115 to 118 in ec15f08

start: frame.line - (frame.context.pre || []).length,
pre: frame.context.pre.join('\n'),
line: frame.context.line,
post: frame.context.post.join('\n')

at this moment, the stack trace is already "cropped" by _parseError :

Math.max(0, lineNumber - (this.codeContext + 1)),

( I didn't really look at how the HTML version worked so sorry if I'm saying something wrong. I'll look more into it tonight. )

@thetutlage
Copy link
Member

Okay got it. So codeContext is already a property which is used internally. So, let's rename this property as we are making it public now.

options = {
  preLines: 2,
  postLines: 2,
}

new Youch(error, request, options)

I see that Youch API can be improved drastically. But for now introducing these two options should do it.

@thetutlage thetutlage merged commit b701a9f into poppinss:develop May 3, 2022
@thetutlage
Copy link
Member

Looks great 👍

@thetutlage
Copy link
Member

Released as 3.2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants