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

doc,test: add information on unwatch(index) command #42882

Closed
wants to merge 3 commits into from

Conversation

cola119
Copy link
Member

@cola119 cola119 commented Apr 27, 2022

unwatch command of node inspect supports to remove specific index element.

unwatch(expr) {
const index = ArrayPrototypeIndexOf(watchedExpressions, expr);
// Unwatch by expression
// or
// Unwatch by watcher number
ArrayPrototypeSplice(watchedExpressions,
index !== -1 ? index : +expr, 1);
},

@nodejs-github-bot nodejs-github-bot added debugger Issues and PRs related to the debugger subsystem. doc Issues and PRs related to the documentations. labels Apr 27, 2022
@cola119 cola119 changed the title doc: add information on unwatch(index) option doc,test: add information on unwatch(index) command Apr 28, 2022
@cola119
Copy link
Member Author

cola119 commented May 9, 2022

@Trott

About #43018 (review), I hope we can discuss here.
unwatch(index), not an expression, is implemented internally but undocumented anywhere. Should I add the doc to node help along with the debugger doc or leave it undocumented in terms of internal use only?

@Trott
Copy link
Member

Trott commented May 9, 2022

If it is clear how to obtain the index and how index differs from expr, and if this would be useful to end users, then we should document it. Thanks.

@cola119
Copy link
Member Author

cola119 commented May 9, 2022

Removing watcher by index would be useful for end users because watchers command prints all watched expressions with their indices.

@cola119
Copy link
Member Author

cola119 commented May 9, 2022

However, the current watch(expr) accepts numeric as well as string expression, which makes the difference index and expr ambiguous.
So I think string validation for watch(expr) is required (#42913).

@cola119 cola119 force-pushed the fix-unwatch-option-doc branch from 9d24260 to 9bcd75c Compare June 19, 2022 12:51
@cola119
Copy link
Member Author

cola119 commented Jun 19, 2022

@lpinca @Mesteery @Trott Could you check again and land if there is nothing wrong with.

@lpinca lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 19, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 19, 2022
@nodejs-github-bot
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debugger Issues and PRs related to the debugger subsystem. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants