Added the extraCRDPChannelPort parameter and updated logs to be generated on %tmpdir%\vscode-chrome-debug.txt #465
Conversation
…ated on %tmpdir%\vscode-chrome-debug_%timestamp%.txt
|
@digeff, |
|
@roblourens This changes need the vscode-chrome-debug-core dependency to be updated to the latest version before this can work... Do you want me to change that too? |
src/chromeDebug.ts
Outdated
| // also note that __dirname here is out/ | ||
| const logFilePath = path.resolve(__dirname, '../vscode-chrome-debug.txt'); | ||
| // non-.txt file types can't be uploaded to github. Generated filename is something like: vscode-chrome-debug_2017-07-19_161210.txt | ||
| const timestampForFilename = new Date().toISOString().replace(/T/, '_').replace(/:/g, '').replace(/\..*/, ''); |
There was a problem hiding this comment.
Is this only needed for your tests? Could the adapter process be started with an environment var giving the name of the log file, or a flag to use the timestamp? Is that reasonable?
This is fine and tmpdir is fine, but I'm still thinking of a way to keep it writing to the same file on each run for vscode.
How will you find the log file? Are you just looking for the newest one in the tmpdir?
There was a problem hiding this comment.
I only added the timestamp to avoid losing the logs the second time we run the debugger. Would appending to the log file rather than starting from scratch work for you?
There was a problem hiding this comment.
No, then it would just get huge. I just prefer overwriting one file but we can keep this if it's what you need.
Technically we could specify this in the launch config, because the file name isn't actually used until the logger is initialized. It's just bad design on my part to set up the hypothetical file name ahead of time. But I don't care enough to change it right now.
There was a problem hiding this comment.
Ok. I'll leave the tmpdir change in this PR (it's the most important change for us), and we might send you a new PR with other logchanges using a launch.json parameter in the future.
There was a problem hiding this comment.
I vaguely remember this coming up partly because of your tests running multiple instances of the adapter at once, and trampling each others' log files. If that's the case, you may not want to strip the milliseconds portion of the timestamp, because if two adapters start within the same second, they'll get the same log file name.
If I'm remembering wrong, ignore it...
|
Yeah could you also update vscode-chrome-debug-core? |
digeff
changed the title
|
I didn't mean you have to revert the timestamp logic for the log file name now - I'll merge this but it's totally ok to keep the timestamp in the name if you want. |
|
I don't want to make your life more difficult, so if we need this we'll add it later as an optional launch.json parameter as you suggested. |
Added the extraCRDPChannelPort parameter and updated logs to be generated on %tmpdir%\vscode-chrome-debug_%timestamp%.txt