Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Default stackTraceDepth to the value from package.json #2200

Merged
merged 1 commit into from
Dec 30, 2018
Merged

Default stackTraceDepth to the value from package.json #2200

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/debugAdapter/goDebug.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ class Delve {
} else if (typeof launchArgs['useApiV1'] === 'boolean') {
this.isApiV1 = launchArgs['useApiV1'];
}
this.stackTraceDepth = launchArgs.stackTraceDepth;
this.stackTraceDepth = launchArgs.stackTraceDepth || 50;
Copy link

Choose a reason for hiding this comment

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

The default value should be probably stored as const to avoid magic number problem.

Copy link
Contributor Author

@segevfiner segevfiner Dec 23, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure where to place the constant since there are no other such constants in the file, should it be:

  1. Somewhere at file scope? If so, where exactly?
  2. Inside the class? (private/public?)
  3. Alternatively, we can import/require/JSON.parse the package.json and get it from there. (import/require will require enabling the TypeScript setting resolveJsonModule)

It's also quite possible that there are other default values strawn around that aren't constant, but I haven't checked.

@ramya-rao-a

Copy link
Contributor

Choose a reason for hiding this comment

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

It's also quite possible that there are other default values strawn around that aren't constant,

Yes, right around this line of code we have set defaults for other properties of the launchArgs variable.

This looks good to me for now. I'll pick up the const issue in some later refactoring

let mode = launchArgs.mode;
let dlvCwd = dirname(program);
let isProgramDirectory = false;
Expand Down