-
-
Notifications
You must be signed in to change notification settings - Fork 625
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
Add Documentation for how to Remotely Debug NVDA #16971
Conversation
WalkthroughThis update enhances the NVDA documentation by introducing a new resource for remote debugging. A link to the "Remote Debugging NVDA" document has been added to the Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant VS as Visual Studio Code
participant NVDA as NVDA Instance
Dev->>VS: Set up remote debugging environment
VS->>Dev: Install required extensions
Dev->>NVDA: Configure NVDA workspace
VS->>NVDA: Establish remote tunnel
NVDA-->>VS: Acknowledge connection
Dev->>VS: Start debugging session
VS->>NVDA: Send debugging commands
NVDA-->>VS: Execute commands and return results
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (11)
projectDocs/dev/remoteDebugging.md (11)
3-5
: Improve clarity and fix minor grammatical issues.Consider rephrasing the sentences for better readability.
- Sometimes it is not possible to effectively debug a running copy of NVDA, particularly if you rely on NVDA or another screen reader for development. - In these cases, it is helpful to use a secondary machine to debug the running copy of NVDA. + Sometimes, it is not possible to effectively debug a running copy of NVDA, especially if you rely on NVDA or another screen reader for development. + In such cases, using a secondary machine to debug the running copy of NVDA can be helpful.
6-6
: Fix spacing issue in the hyperlink.Remove the space between the link text and the URL.
- This document explains how to use [Visual Studio Code] (VS Code) to remotely debug NVDA. + This document explains how to use [Visual Studio Code](VS Code) to remotely debug NVDA.
19-19
: Fix typo in the sentence.Correct the typo "canuse" to "can use".
- Alternatively, you canuse the [VS Code Standalone CLI]. + Alternatively, you can use the [VS Code Standalone CLI].
12-13
: Improve readability and fix grammatical issues.Consider rephrasing the sentences for better readability.
- To remotely debug NVDA, you must have the debug extension for the language you want to debug installed--[Python Debugger] for python and [C/C++] for C++. - The [Python C++ Debugger] is recommended for debugging Python and C++ at the same time. + To remotely debug NVDA, you must have the debug extension for the language you want to debug installed—[Python Debugger] for Python and [C/C++] for C++. + The [Python C++ Debugger] is recommended for debugging both Python and C++ simultaneously.
14-14
: Fix grammatical issue.Consider rephrasing the sentence for better readability.
- Consider using the pre-configured NVDA VS Code workspace, which will recommend all required addons, and has other required settings configured. + Consider using the pre-configured NVDA VS Code workspace, which recommends all required add-ons and has other necessary settings configured.
21-23
: Improve readability and fix grammatical issues.Consider rephrasing the sentences for better readability.
- If you do not have (or do not want to use) two physical machines, you can use a virtual machine as the secondary machine. - Depending on the virtualisation software (hypervisor) in use, this may require hardware virtualisation support. - Without hardware virtualisation support, using a virtual machine will likely be slow, or may not be possible. + If you do not have (or do not want to use) two physical machines, you can use a virtual machine as the secondary machine. + Depending on the virtualization software (hypervisor) in use, this may require hardware virtualization support. + Without hardware virtualization support, using a virtual machine will likely be slow or may not be possible.
34-38
: Fix grammatical issues and improve clarity.Consider rephrasing the sentences for better readability.
- Set up a Remote tunnel so that another VS code can control this VS code: - Open the Command Palette (`Ctrl`+`Shift`+`P`) and select "Remote Tunnels: Turn on Remote Tunnel Access...". - Choose whether you want the tunnel to be open for this session (only when VS Code is open), or as a service (whenever you're logged in to this machine). - Installing the tunnel as a service will result in the tunnel continuing to remain active in the background. - This option is not recommended. + Set up a Remote tunnel so that another instance of VS Code can control this one: + Open the Command Palette (`Ctrl`+`Shift`+`P`) and select "Remote Tunnels: Turn on Remote Tunnel Access...". + Choose whether you want the tunnel to be open for this session (only when VS Code is open) or as a service (whenever you're logged in to this machine). + Installing the tunnel as a service will keep it active in the background. + This option is not recommended.Tools
LanguageTool
[style] ~37-~37: Consider using “remaining”.
Context: ... as a service will result in the tunnel continuing to remain active in the background. This ...(CONTINUE_TO_REMAIN)
55-57
: Fix grammatical issues and improve clarity.Consider rephrasing the sentences for better readability.
- Choose the account type you used to publish the tunnel from your secondary machine. - If you're not already logged in, log in when asked. - Make sure you log in to the same account you used to publish the tunnel. + Choose the account type you used to publish the tunnel from your secondary machine. + If you're not already logged in, log in when prompted. + Ensure you log in to the same account you used to publish the tunnel.
61-62
: Fix typo and improve readability.Correct the typo "procede" to "proceed" and rephrase the sentence for better readability.
- Even if you have the required extensions installed locally, VS Code may prompt you to install them in the remote workspace. - If prompted, procede with the installation of the extensions. + Even if you have the required extensions installed locally, VS Code may prompt you to install them in the remote workspace. + If prompted, proceed with the installation of the extensions.
75-79
: Improve readability and fix grammatical issues.Consider rephrasing the sentences for better readability.
- On your secondary machine, open the Command Palette (`Ctrl`+`Shift`+`P`) and run "Remote Tunnels: Turn off Remote Tunnel Access...". - Press "Yes" when prompted. - If you chose to create a session tunnel, closing VS Code without turning off remote tunnel access will close the tunnel until you open VS Code again. - If you created the tunnel as a service, the tunnel will continue to remain active in the background until you turn off remote tunnel access. + On your secondary machine, open the Command Palette (`Ctrl`+`Shift`+`P`) and run "Remote Tunnels: Turn off Remote Tunnel Access...". + Press "Yes" when prompted. + If you created a session tunnel, closing VS Code without turning off remote tunnel access will close the tunnel until you open VS Code again. + If you created the tunnel as a service, it will remain active in the background until you turn off remote tunnel access.Tools
LanguageTool
[style] ~79-~79: Consider using “remain”.
Context: ...he tunnel as a service, the tunnel will continue to remain active in the background until you turn...(CONTINUE_TO_REMAIN)
80-81
: Fix repetitive sentence structure.Rephrase the sentences to avoid repetition.
- If you created the tunnel at the CLI, `Ctrl`+`C` to close it. - If you created it as a service, run `code tunnel kill` to stop the service, or `code tunnel service uninstall` to remove it. + If you created the tunnel at the CLI, use `Ctrl`+`C` to close it. + If you created it as a service, run `code tunnel kill` to stop the service or `code tunnel service uninstall` to remove it.Tools
LanguageTool
[style] ~80-~80: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... you turn off remote tunnel access. * If you created the tunnel at the CLI, `Ctr...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~81-~81: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...at the CLI,Ctrl
+C
to close it. If you created it as a service, run `code ...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking very thorough. Just a couple of places you may have confused development machine / secondary machine.
Also, it might be an idea to include a brief example of a debug session. Perhaps just the steps to insert a breakpoint on script_dateTime in globalCommands.py, launch NVDA (Python) with f5, and once NvDA is running, press NVDA+f12 to report the time, which will hit the breakpoint, and then you can look at the call stack for the main thread.
I'm really exited about this possibility! I wonder, should we actually reconsider bundling the workspace in the NVDA repository itself?
The only downside I can think of, is that users are less able to perform changes in the workspace. However this should easily be fixable by keeping .vscode in .gitignore by defualt. |
Fixed up a few cases where development and secondary machine were confused. Co-authored-by: Michael Curran <[email protected]>
@SaschaCowley please fix spelling of "debug" in title. :) |
@michaelDCurran Thanks for the review. I've added example debugging steps. @LeonarddeR We have been thinking about whether to add the VS Code workspace as a submodule or include it in the repo directly, but have not reached a decision yet. @XLTechie Was the typo just in the title PR? If so it's fixed now :) |
@LeonarddeR, @XLTechie (and others), if you get a chance to try out these instructions to make sure I haven't missed anything before we merge, I'd really appreciate it! :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example looks good. Just two small things I think we should specifically call out.
I'm not a VS Code user, sorry. (And yes, just the title).
CC @nvdaes
|
@XLTechie , I don't know if I'll be able to test this before it's merged, but I'll try to do it. |
@SaschaCowley If you want to merge this sooner than later, I"m afraid I won't be able to test this as I currently don't have a testing environment at hand. |
@LeonarddeR When choosing what to use for your development and secondary machines, it is worth considering the development (controlling) machine should have a physical keyboard that cannot be ever locked up by a an NVDA being debugged. I.e. If using a VM or sandbox for the development (controlling) machine, the VM should access a physical keyboard exclusively, and not depend whether the VM is in focus or not. |
Co-authored-by: Michael Curran <[email protected]>
See test results for failed build of commit 7e3317723d |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. But we'll want to merge nvaccess/vscode-nvda#16 right at the same time.
I've tested this and I think that the documentation is right. |
Fair point, I hadn't considered that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you please fix up the shortcut key descriptions e.g. control+alt+o
to follow the user guide standards?
Note, I haven't performed a more thorough review yet.
Co-authored-by: Sean Budd <[email protected]>
Great work here, this will be very useful for devs |
Co-authored-by: Sean Budd <[email protected]>
0724801
to
bd095e6
Compare
Link to issue number:
None.
Related: nvaccess/vscode-nvda#16
Summary of the issue:
Sometimes it is not possible to debug certain issues with NVDA on one computer, especially when you rely on NVDA to access the computer.
Description of user facing changes
None.
Description of development approach
Tried a couple of approaches to remotely debugging NVDA, with @michaelDCurran's help. Documented the best solution, including thoroughly testing from scratch to test assumptions made.
Testing strategy:
Ran through documented steps from scratch to try and eliminate assumptions.
Known issues with pull request:
None.
Code Review Checklist:
Summary by CodeRabbit
New Features
Documentation