-
Notifications
You must be signed in to change notification settings - Fork 510
(GH-1437) Fix detecting contiguous comment blocks and regions #1438
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
(GH-1437) Fix detecting contiguous comment blocks and regions #1438
Conversation
Previously the region comment detection used a little convoluted method to detect regions in a document. This commit simplifies the detection by extracting the line from the document and using regex's similar to that used by the PowerShell language configuration. This also removes the need for the emptyline and subsequentText method calls. While the performance of the folder is pretty quick, this should in theory make it faster on larger documents by doing less calls to the VSCode Document API.
Previously the syntax folding feature would not correctly identify comment blocks and comment regions if they appeared all together. This commit changes the comment block detection to ignore line comments that start with region and endregion, i.e. region block start/end directives. This commit also adds test for this scenario.
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 great Glenn!
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.
LGTM!
The following was folding previously but not since the extension assume the folding logic.
|
@cadayton It would be better if you raised an issue (https://github.com/PowerShell/vscode-powershell/issues/new/choose) instead of commenting on this PR. |
@cadayton this is because we've switched away from indentation-based folding to syntax-based folding. A work around for you would be to use Alternatively, you can simply disable syntax-based folding which will fall back to indentation-based folding. (not at my computer at the moment so I don't have the exact name but just search for "folding" in the settings. I'm not sure we really want to support syntax-based folding and indentation-based folding at the same time so I would really encourage you to do one of the above workarounds. |
Like Glenn said, feel free to open an issue if you feel otherwise! 👍 |
Thanks Tyler.
I'll try the workaround, but have quite few references that need to be modified to switch over. Not to mention several GitHub projects that need to change too. Prior to using VS code, I used PowerGUI which required #region & #endregion too and after switching over to VS Code I converted everything to syntax folding since it worked. It is nice to avoid adding #region and $endregion, but understand if it is making it harder for VS Code parser too difficult to manage.
Looks like I need to switch back now. :(
Craig Dayton
[email protected]
360-466-8778
Sent with [ProtonMail](https://protonmail.com) Secure Email.
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
…On August 6, 2018 6:55 AM, Tyler James Leonhardt ***@***.***> wrote:
***@***.***(https://github.com/cadayton) this is because we've switched away from indentation-based folding to syntax-based folding.
A work around for you would be to use #region Initialize Routine and #endregion. This is recommended.
Alternatively, you can simply disable syntax-based folding which will fall back to indentation-based folding. (not at my computer at the moment so I don't have the exact name but just search for "folding" in the settings.
I'm not sure we really want to support syntax-based folding and indentation-based folding at the same time so I would really encourage you to do one of the above workarounds.
—
You are receiving this because you were mentioned.
Reply to this email directly, [view it on GitHub](#1438 (comment)), or [mute the thread](https://github.com/notifications/unsubscribe-auth/AFVDGvehACzxzUnfa9Mzcwy9aweTpvtEks5uOErNgaJpZM4VQiI4).
|
PR Summary
Previously the region comment detection used a little convoluted method to
detect regions in a document. This commit simplifies the detection by
extracting the line from the document and using regex's similar to that used by
the PowerShell language configuration. This also removes the need for the
emptyline and subsequentText method calls. While the performance of the folder
is pretty quick, this should in theory make it faster on larger documents by
doing less calls to the VSCode Document API.
Previously the syntax folding feature would not correctly identify comment
blocks and comment regions if they appeared all together. This commit changes
the comment block detection to ignore line comments that start with region and
endregion, i.e. region block start/end directives. This commit also adds test
for this scenario.
Fixes #1437
PR Checklist
Note: Tick the boxes below that apply to this pull request by putting an
x
between the square brackets.Please mark anything not applicable to this PR
NA
.WIP:
to the beginning of the title and remove the prefix when the PR is ready