Skip to content

Add Missing Preference Variables #178

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

Closed
wants to merge 1 commit into from
Closed

Add Missing Preference Variables #178

wants to merge 1 commit into from

Conversation

glachancecmaisonneuve
Copy link
Contributor

@glachancecmaisonneuve glachancecmaisonneuve commented Jun 1, 2019

The committer may wonder at the seemingly random position and word order used in that keyword enumeration while the other keyword enumerations are alphabetic. The word are ordered according to the about_preference_variables page.

Keywords added:

  • LogCommandHealthEvent
  • LogCommandLifecycleEvent
  • LogEngineHealthEven
  • LogEngineLifecycleEvent
  • LogProviderLifecycleEvent
  • LogProviderHealthEvent
  • PSModuleAutoLoadingPreference
  • InformationPreference

@glachancecmaisonneuve glachancecmaisonneuve changed the title InformationPreference Support Preferences Variables Highlighting Jun 1, 2019
@glachancecmaisonneuve glachancecmaisonneuve changed the title Preferences Variables Highlighting Preference Variables Highlighting Jun 1, 2019
@msftrncs
Copy link
Contributor

msftrncs commented Jun 2, 2019

A couple thoughts:

  • The incorrect (out of alphabetic) order in the documentation is probably an error and should be fixed, which means at some time in the future, the order in the grammar file would still be confusing for people until they looked up this PR.
  • Not sure that there is any preference for PR titles, but it might be beneficial to add to title that this PR Add Missing.
  • I find that PSModuleAutoLoadingPreference was already there, just without the L of loading capitalized, probably should not mention that it was 'added'. Since the change makes no functional difference I am not sure it needs to be mentioned.
  • Might add all these variables to the https://github.com/PowerShell/EditorSyntax/blob/master/spec/testfiles/syntax_test_TheBigTestFile.ps1 test file with the appropriate scope tests as to insure these don't regress in the future. There are probably a lot more variables that are missing test conditions, but that's probably the scope of another PR.

Also, if not aware, PSScriptAnalyzer flags assignments to the 'Log...' variables with PSScriptAnalyzer(PSUseDeclaredVarsMoreThanAssignments) if you don't also utilize the variables elsewhere. It doesn't do this for the other preference variables. Evidently these variables have been forgotten about across the board. They also do not come up in completion suggestions in VS Code/PowerShell extension, which is kind of odd since they do come up in completion in a PWSH console, which is where VS Code/PowerShell extension is supposed to get its completions from.

@glachancecmaisonneuve glachancecmaisonneuve changed the title Preference Variables Highlighting Add Missing Preference Variables Highlighting Jun 4, 2019
@glachancecmaisonneuve glachancecmaisonneuve changed the title Add Missing Preference Variables Highlighting Add Missing Preference Variables Jun 4, 2019
@glachancecmaisonneuve
Copy link
Contributor Author

glachancecmaisonneuve commented Jun 4, 2019

  • Not sure that there is any preference for PR titles, but it might be beneficial to add to title that this PR Add Missing.

Done. Thank you.

  • I find that PSModuleAutoLoadingPreference was already there, just without the L of loading capitalized, probably

Done, Thank you, totally missed it.

  • Might add all these variables to the syntax_test_TheBigTestFile.ps1 test file with the appropriate scope tests

Some help would be appreciated here. It looks like the TheBigTestFile is generated with powershell-spec.coffee, however coffescript is not installed with the other packages so that looks like a dead end. The appveyor build seems to give some clues as how to proceed, i.e. something to do with atom, but my investigation comes to halt there with the ParseJasmin function that, I observed, when traced, has the interesting side effect of making the reader have an irresistible craving for mint flavoured ice-cream (7 times out of ten) or even more interestingly, chocolate covered peanuts (3 times out of ten). One time out of a hundred however, the effect is rather startling, the reader gets an overwhelming urge to yell "I just wanted "InformationPreference" to be the same colour as the rest!". A few tentative searches on other global variables reveal nothing interesting.

Also, if not aware, PSScriptAnalyzer flags assignments to the 'Log...' variables with PSScriptAnalyzer(PSUseDeclaredVarsMoreThanAssignments) if you don't also utilize the variables elsewhere. It doesn't do this for the other preference variables. Evidently these variables have been forgotten about across the board. They also do not come up in completion suggestions in VS Code/PowerShell extension, which is kind of odd since they do come up in completion in a PWSH console, which is where VS Code/PowerShell extension is supposed to get its completions from.

Non prescriptive. Not addressed.

@msftrncs
Copy link
Contributor

msftrncs commented Jun 5, 2019

Be aware there are two 'thebigtestfiles'.

The tests are stored in https://github.com/PowerShell/EditorSyntax/blob/master/spec/testfiles/syntax_test_TheBigTestFile.ps1. The other one is mostly used for examples, specially for items that have yet to to work correctly.

The spec.coffee file you mention contains a description for the test engine, 'atom-grammar-test', which is an add-on to 'Atom', GitHub's own TextEditor project.

The 'build.ps1' build file will download and install Atom and the test bundle, and generates a JSON grammar file for Atom to use for the tests. Running the build script does require NPM/Node, as currently the JSON build process is in JavaScript.

I don't know anything about Jasmine, it appears its the test result output format, and there is a build-tool that reformats it for easier reading.

Use .\build.ps1 -test to build and then test the grammar file, which includes downloading and installing Atom as a dependency along with a couple other support repositories.

Variables:
LogCommandHealthEvent
LogCommandLifecycleEvent
LogEngineHealthEvent
LogEngineLifecycleEvent
LogProviderLifecycleEvent
LogProviderHealthEvent
InformationPreference

Attributes:
SupportsWildcards
@glachancecmaisonneuve
Copy link
Contributor Author

glachancecmaisonneuve commented Jul 27, 2019

This should be good2go. Added tests.

@TylerLeonhardt I may have merged one too many commits into this one (the previous SupportsWildcard attribute). Feel free to commit the changes yourself, or ask me to redo properly

@glachancecmaisonneuve
Copy link
Contributor Author

I messed this one up. I'll restart PR

@glachancecmaisonneuve glachancecmaisonneuve deleted the InformationPreferenceSupport branch July 28, 2019 16:35
@glachancecmaisonneuve
Copy link
Contributor Author

follow up: #180

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.

2 participants