Skip to content

Add new snippet for #region #1368

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

Merged

Conversation

lipkau
Copy link
Contributor

@lipkau lipkau commented Jun 14, 2018

PR Summary

Added a new snippet to the module and the community snippets for creating a new region block:

#region
#endregion

PR Checklist

  • PR has a meaningful title
  • 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

@msftclas
Copy link

msftclas commented Jun 14, 2018

CLA assistant check
All CLA requirements met.

"prefix": "#region",
"body": [
"#region ${1}",
"${2}",
Copy link
Member

@TylerLeonhardt TylerLeonhardt Jun 14, 2018

Choose a reason for hiding this comment

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

Do people typically put the region tag after #endregion?

I've always seen:

#region foo

#endregion

vs

#region foo

#endregion foo

I'm not sure

Copy link
Contributor

@rjmholt rjmholt Jun 14, 2018

Choose a reason for hiding this comment

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

In C# the most common form I've seen is:

#region Private variables

private readonly IEnumerable<string> _strs = new [] { "hello", "goodbye" };

private string _greeting = "Aloha";

#endregion // Private variables

I would imagine the PowerShell version of that is:

#region My region
...
#endregion # My region

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess in PowerShell the region is already commented out, but that's an implementation detail from the line-comment delimiter and the pragma start-char being the same in my mind.

Copy link
Contributor

@rkeithhill rkeithhill Jun 14, 2018

Choose a reason for hiding this comment

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

My first pref (by a smidge) would be:

#endregion

Second would be:

#endregion <name>

But either would be fine by me.

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like if both @tylerl0706 and @rkeithhill agree on a first preference then it should be that 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

@@ -1,3 +1,5 @@
<!-- markdownlint-disable MD024 -->
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps we should move this to the .vscode/settings.json file? This would probably apply to the CHANGELOG as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This slipped in by accident.
I am waiting on the merge of this one to create the next with this line --> which I just moved to settings.json

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.

LGTM! :)

@TylerLeonhardt TylerLeonhardt merged commit 7c57929 into PowerShell:master Jun 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants