Skip to content

Conversation

kapilmb
Copy link

@kapilmb kapilmb commented Jan 12, 2017

No description provided.

Kapil Borle added 5 commits January 11, 2017 19:05
This enables the client to send settings as a hashtable instead of pointing to a settings file for script analysis. Our current code structure allows us to pass either a set of rules or a settings file path but not both. However, to provide code formatting settings we not only need to send a settings file to configure the given rules but also the rules in a particular order. The best approach in this case is to update and send a settings hashtable everytime we request code formatting markers.
@daviwil
Copy link
Contributor

daviwil commented Jan 13, 2017

Oh, I just realized there's a build failure here in the PowerShell v3 and v4 API tests:

Language\LanguageService.cs(480,58): error CS1061: 'HashtableAst' does not contain a definition for 'SafeGetValue' and no extension method 'SafeGetValue' accepting a first argument of type 'HashtableAst' could be found

@kapilmb
Copy link
Author

kapilmb commented Jan 13, 2017

Yes, even I noticed it only a couple of minutes ago. PSv3 and PSv4 do not have the ast.safegetvalue() api. For PSSA we wrote our own hashtable parser to support PSv3 and PSv4. But I do not think that is a very good idea. Another approach is to transmit an object instead of hashtable literal and convert it to hashtable at the server.

@daviwil
Copy link
Contributor

daviwil commented Jan 13, 2017

Damn, I think I had written a review comment about this in the past but I might have deleted it. Here's what you can do:

  • Change ScriptFileMarkerRequestParams.settings to a JObject instead of string
  • When you want to convert to a Hashtable, use the JObject instance method ToObject to convert to that type
  • On the client side, instead of serializing your settings object into a string just pass it directly in the settings property when you make the request.

JObject comes from Newtonsoft.Json.Linq

@daviwil
Copy link
Contributor

daviwil commented Jan 14, 2017

Seems like we're still getting the build failures from the use of HashtableAst.SafeGetValue, is this code needed anymore?

: (hashtableAst as HashtableAst).SafeGetValue() as Hashtable;

@kapilmb
Copy link
Author

kapilmb commented Jan 17, 2017

I forgot to remove the redundant method. The tests now don't fail on compilation for v3 and v4.

@daviwil
Copy link
Contributor

daviwil commented Jan 17, 2017

Awesome, thanks a lot! Merging it.

@daviwil daviwil merged commit f8f6f32 into develop Jan 17, 2017
@daviwil daviwil deleted the kapilmb/code-formatting branch January 17, 2017 20:02
@kapilmb
Copy link
Author

kapilmb commented Jan 17, 2017

@daviwil Thanks!

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.

3 participants