Skip to content

Conversation

@MrMineO5
Copy link
Contributor

@MrMineO5 MrMineO5 commented Aug 6, 2022

No description provided.

@BFergerson BFergerson enabled auto-merge August 6, 2022 15:28
@MrMineO5
Copy link
Contributor Author

MrMineO5 commented Aug 6, 2022

Your test has the : in c:/, which causes an exception when resolving the line number from the source string. I remove that : in the nodejs probe so it ends up as /c/path instead of file:///c:/path

It may be better to have the line number also as a separate field, so if the source happens to contain a :, it isn't an issue

@BFergerson BFergerson merged commit 4e543ce into sourceplusplus:master Aug 6, 2022
@BFergerson
Copy link
Contributor

I took that stack trace off StackOverflow to use a real example. I looked around for other examples but they all looked the same. If you have a legit stack trace that this won't work for we can expand the logic. I don't think you should manipulate the stack trace to make it easier to parse. I'd like this logic to work on the original data so it can be used elsewhere.

Please give provide example stack traces that don't work and I'll add tests.

@MrMineO5
Copy link
Contributor Author

MrMineO5 commented Aug 6, 2022

I think calling LiveStackTraceElement#sourceAsLineNumber on that stack trace should throw an exception, since it takes everything after the first :

@BFergerson
Copy link
Contributor

BFergerson commented Aug 6, 2022

Ah, I see what you mean. I'll expand the tests, 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.

2 participants