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

Convert parse-stack.js to TypeScript #10653

Open
wants to merge 7 commits into
base: release-1.8.2
from

Conversation

@menewman
Copy link

commented Jul 27, 2019

A small TypeScript conversion. For markTop and markBottom I took the coward's way out and just used @ts-ignore so that I wouldn't have to "fix" the usage of implicit this, since I wasn't confident that no consumers relied on that behavior (although I couldn't find any specific examples where it was needed).

menewman added 7 commits Jul 23, 2019
Merge pull request #1 from meteor/release-1.8.2
Update Release 1.8.2 branch from master repo
Merge pull request #2 from meteor/release-1.8.2
update fork from origin
@jamesmillerburgess
Copy link
Contributor

left a comment

LGTM!

func: m[1],
file: m[5],
line: m[7] ? +m[7] : undefined,
column: m[9] ? +m[9] : undefined
line: m[7] ? +m[7] : null,

This comment has been minimized.

Copy link
@jamesmillerburgess

jamesmillerburgess Aug 6, 2019

Contributor

🆗 I wondered if these changes from undefined to null would be safe, and it looks ok, because the checks in buildmessage.js are for thruthiness:

if (message.line) {
line += ":" + message.line;
if (message.column) {
// XXX maybe exclude unless specifically requested (eg,
// for an automated tool that's parsing our output?)
line += ":" + message.column;
}
}

if (message.func && stack.length <= 1) {
line += " (at " + message.func + ")";
}

This comment has been minimized.

Copy link
@menewman

menewman Aug 6, 2019

Author

thanks for double-checking and confirming!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.