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.

Sign up for GitHub

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

Jump to bottom

(GH-1514) Show the last line of folding regions as per default VSCode #1557

Merged
merged 1 commit into from
Oct 23, 2018
Merged

(GH-1514) Show the last line of folding regions as per default VSCode #1557

merged 1 commit into from
Oct 23, 2018

Conversation

glennsarti
Copy link
Contributor

@glennsarti glennsarti commented Oct 2, 2018 •
edited
Loading

PR Summary

Previously the code folding provider would hide the last line of the region,
however the default VS Code code folding behaviour is to instead hide from
the beginning to end minus one, lines. This commit;

  • Adds a configuration parameter to show/hide the last line in a region,
    defaulting to Show the last line
  • Modifies conversion method for the internal region class to FoldingRange
    object to show/hide the last line based on the extension settings
  • Modifies the tests for the default value and adds tests for the show and hide
    scenarios

This code is based on a solution created by;
ens-rpitcher russell.pitcher@attenda.com

Fixes #1514

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.

  • PR has a meaningful title
  • PR has tests
  • Summarized changes
  • This PR is ready to merge and is not work in progress
    • If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready

@glennsarti glennsarti mentioned this pull request Oct 2, 2018
Closed
rjmholt
rjmholt reviewed Oct 8, 2018
View reviewed changes
src/features/Folding.ts Outdated
public toFoldingRange(settings: Settings.ISettings): vscode.FoldingRange {
if (settings.codeFolding && settings.codeFolding.showLastLine) {
return new vscode.FoldingRange(this.startline, this.endline - 1, this.rangeKind);
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

This else is unneeded -- the top clause returns

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed, although I would've though TSlint would pickup on that...

src/features/Folding.ts Show resolved Hide resolved
test/features/folding.test.ts Show resolved Hide resolved
@rjmholt
Copy link
Contributor

rjmholt commented Oct 8, 2018

Sorry I got back to you on this so late @glennsarti, just left a couple of comments, but this looks pretty good to me

@glennsarti glennsarti mentioned this pull request Oct 9, 2018
Closed
@glennsarti
(PowerShellGH-1514) Show the last line of folding regions as per defa…
af7cda6 
…ult VSCode

Previously the code folding provider would hide the last line of the region,
however the default VS Code code folding behaviour is to instead hide from
the beginning to end minus one, lines.  This commit;

* Adds a configuration parameter to show/hide the last line in a region,
  defaulting to Show the last line
* Modifies conversion method for the internal region class to FoldingRange
  object to show/hide the last line based on the extension settings
* Modifies the tests for the default value and adds tests for the show and hide
  scenarios

This code is based on a solution created by;
ens-rpitcher <russell.pitcher@attenda.com>
@glennsarti
Copy link
Contributor Author 

For a single document
    √ Can detect all of the foldable regions in a document with CRLF line endings (120ms)
    √ Can detect all of the foldable regions in a document with LF line endings (45ms)
    √ Can detect all of the foldable regions in a document with mismatched regions (306ms)
    √ Does not return duplicate or overlapping regions
    Where showLastLine setting is false
      √ Can detect all foldable regions in a document

Light is green

rjmholt
rjmholt approved these changes Oct 10, 2018
View reviewed changes
Copy link
Contributor

@rjmholt rjmholt left a comment

Choose a reason for hiding this comment

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

LGTM!

rkeithhill
rkeithhill approved these changes Oct 11, 2018
View reviewed changes
Copy link
Contributor

@rkeithhill rkeithhill left a comment

Choose a reason for hiding this comment

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

LGTM

TylerLeonhardt
TylerLeonhardt approved these changes Oct 23, 2018
View reviewed changes
Copy link
Member

@TylerLeonhardt TylerLeonhardt left a comment

Choose a reason for hiding this comment

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

🚢 it 😎

@TylerLeonhardt TylerLeonhardt merged commit db76d05 into PowerShell:master Oct 23, 2018
@TylerLeonhardt TylerLeonhardt mentioned this pull request Oct 23, 2018
Closed
3 tasks
@rjmholt rjmholt mentioned this pull request Nov 8, 2018
Closed
glennsarti added a commit to glennsarti/PowerShellEditorServices that referenced this pull request Nov 12, 2018
@glennsarti
(PowerShellGH-793)) Add support for Folding Range client settings
90a4efb 
This commit uses the the client configuration settings to modify the behaviour
of the server. In particular the Enable and ShowLastLine setting.

* The enable setting will change the provider to return null when disabled
* The ShowLastLine setting emulates the behaviour in PowerShell/vscode-powershell#1557
  * Modifies conversion method for the internal region class to FoldingRange
    object to show/hide the last line based on the extension settings
  * Modifies the tests for the default value and adds tests for the show and hide
    scenarios
glennsarti added a commit to glennsarti/PowerShellEditorServices that referenced this pull request Nov 13, 2018
@glennsarti
(PowerShellGH-793) Add support for Folding Range client settings
b86ce2b 
This commit uses the the client configuration settings to modify the behaviour
of the server. In particular the Enable and ShowLastLine setting.

* The enable setting will change the provider to return null when disabled
* The ShowLastLine setting emulates the behaviour in PowerShell/vscode-powershell#1557
  * Modifies conversion method for the internal region class to FoldingRange
    object to show/hide the last line based on the extension settings
  * Modifies the tests for the default value and adds tests for the show and hide
    scenarios
glennsarti added a commit to glennsarti/PowerShellEditorServices that referenced this pull request Nov 13, 2018
@glennsarti
(PowerShellGH-793) Add support for Folding Range client settings
691988a 
This commit uses the the client configuration settings to modify the behaviour
of the server. In particular the Enable and ShowLastLine setting.

* The enable setting will change the provider to return null when disabled
* The ShowLastLine setting emulates the behaviour in PowerShell/vscode-powershell#1557
  * Modifies conversion method for the internal region class to FoldingRange
    object to show/hide the last line based on the extension settings
  * Modifies the tests for the default value and adds tests for the show and hide
    scenarios
glennsarti added a commit to glennsarti/PowerShellEditorServices that referenced this pull request Nov 14, 2018
@glennsarti
(PowerShellGH-793) Add support for Folding Range client settings
61014e1 
This commit uses the the client configuration settings to modify the behaviour
of the server. In particular the Enable and ShowLastLine setting.

* The enable setting will change the provider to return null when disabled
* The ShowLastLine setting emulates the behaviour in PowerShell/vscode-powershell#1557
  * Modifies conversion method for the internal region class to FoldingRange
    object to show/hide the last line based on the extension settings
  * Modifies the tests for the default value and adds tests for the show and hide
    scenarios
glennsarti added a commit to glennsarti/PowerShellEditorServices that referenced this pull request Nov 14, 2018
@glennsarti
(PowerShellGH-793) Add support for Folding Range client settings
72dcd9d 
This commit uses the the client configuration settings to modify the behaviour
of the server. In particular the Enable and ShowLastLine setting.

* The enable setting will change the provider to return null when disabled
* The ShowLastLine setting emulates the behaviour in PowerShell/vscode-powershell#1557
  * Modifies conversion method for the internal region class to FoldingRange
    object to show/hide the last line based on the extension settings
  * Modifies the tests for the default value and adds tests for the show and hide
    scenarios
glennsarti added a commit to glennsarti/PowerShellEditorServices that referenced this pull request Nov 16, 2018
@glennsarti
(PowerShellGH-793) Add support for Folding Range client settings
0aa26c4 
This commit uses the the client configuration settings to modify the behaviour
of the server. In particular the Enable and ShowLastLine setting.

* The enable setting will change the provider to return null when disabled
* The ShowLastLine setting emulates the behaviour in PowerShell/vscode-powershell#1557
  * Modifies conversion method for the internal region class to FoldingRange
    object to show/hide the last line based on the extension settings
  * Modifies the tests for the default value and adds tests for the show and hide
    scenarios
glennsarti added a commit to glennsarti/PowerShellEditorServices that referenced this pull request Nov 16, 2018
@glennsarti
(PowerShellGH-793) Add support for Folding Range client settings
8ea8606 
This commit uses the the client configuration settings to modify the behaviour
of the server. In particular the Enable and ShowLastLine setting.

* The enable setting will change the provider to return null when disabled
* The ShowLastLine setting emulates the behaviour in PowerShell/vscode-powershell#1557
  * Modifies conversion method for the internal region class to FoldingRange
    object to show/hide the last line based on the extension settings
  * Modifies the tests for the default value and adds tests for the show and hide
    scenarios
glennsarti added a commit to glennsarti/PowerShellEditorServices that referenced this pull request Nov 22, 2018
@glennsarti
(PowerShellGH-793) Add support for Folding Range client settings
c23658f 
This commit uses the the client configuration settings to modify the behaviour
of the server. In particular the Enable and ShowLastLine setting.

* The enable setting will change the provider to return null when disabled
* The ShowLastLine setting emulates the behaviour in PowerShell/vscode-powershell#1557
  * Modifies conversion method for the internal region class to FoldingRange
    object to show/hide the last line based on the extension settings
  * Modifies the tests for the default value and adds tests for the show and hide
    scenarios
glennsarti added a commit to glennsarti/PowerShellEditorServices that referenced this pull request Nov 22, 2018
@glennsarti
(PowerShellGH-793) Add support for Folding Range client settings
77aa878 
This commit uses the the client configuration settings to modify the behaviour
of the server. In particular the Enable and ShowLastLine setting.

* The enable setting will change the provider to return null when disabled
* The ShowLastLine setting emulates the behaviour in PowerShell/vscode-powershell#1557
  * Modifies conversion method for the internal region class to FoldingRange
    object to show/hide the last line based on the extension settings
  * Modifies the tests for the default value and adds tests for the show and hide
    scenarios
rjmholt pushed a commit to PowerShell/PowerShellEditorServices that referenced this pull request Nov 28, 2018
@glennsarti @rjmholt
Add syntax folding (#777)
fbe8871 
* Add simple folding range provider capability
* Uses a similar method to that implemented in the VS Code extension
* Uses the PowerShell tokeniser instead of the AST.  This is due to the AST
  ignoring comment sections.  Without the comment parsing folding for #region
  etc. will not work
* uses the the client configuration settings to modify the behaviour
of the server. In particular the Enable and ShowLastLine setting.
* The enable setting will change the provider to return null when disabled
* The ShowLastLine setting emulates the behaviour in PowerShell/vscode-powershell#1557
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rjmholt rjmholt rjmholt approved these changes

@rkeithhill rkeithhill rkeithhill approved these changes

@TylerLeonhardt TylerLeonhardt TylerLeonhardt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Folding: Allow option to leave closing brace line visible
4 participants
@glennsarti @rjmholt @TylerLeonhardt @rkeithhill