Update Jsrt Utf8 types and naming

[obastemur]

No description provided.

[obastemur]

Fixes #1918

[jianchun]

Could you elaborate why we need this #ifdef (either NTBUILD or CHAKRACOREBUILD_)? My understanding is that ChakraCore.h is the header for ChakraCore only thus is always CHAKRACOREBUILD. NTBUILD does not use this header at all.

[obastemur]

We had discussed this over emails. NTBUILD naming gives a wrong impression.

[jianchun]

We had discussed this over emails. NTBUILD naming gives a wrong impression.

That was not my question. I was saying nothing was needed.

[obastemur]

I'm not sure when and where we had discussed this but I had brought this ifdef after a discussion. I can happily remove it.

[jianchun]

Unless you find out why you needed these, removing them seems cleaner to me. (The original design did not require #ifdef -- "ChakraCommon.h" was supposed to hold APIs common to Core and Full. "ChakraCore.h" was supposed to hold new ChakraCore APIs which might later be promoted to "ChakraCommon.h".)

[obastemur]

Agree. Instead of maintaining this, will remove it for good.

[jianchun]

CHAKRACOREBUILD_ is a bit of uncommon style (_ at the end). Does other option work (CHAKRACORE_BUILD, _CHAKRACORE_BUILD)?

[obastemur]

The styling was @curtisman 's choice and I also liked it. The _ at the end gives private use only impression.

[jianchun]

The styling was @curtisman 's choice and I also liked it. The _ at the end gives private use only impression.

Usually we use leading _ to indicate private use.

[jianchun]

Do we plan to consolidate JsDiagEvaluate to take JsValueRef expression?

@liminzhu @agarwal-sandeep

[agarwal-sandeep]

@jianchun @obastemur Yes we should, I had same feedback when we were doing xplat string APIs and I was assuming this PR would cover that. @obastemur can we open an issue

[obastemur]

@agarwal-sandeep @jianchun I didn't update JsDiagEvaluateUtf8 to JsDiagEvaluate since we already have one for ChakraFull with same name. As for the JsValueRef expression, what is the suggestion here?

[agarwal-sandeep]

@liminzhu Since JsDiagEvaluate is new API should we only have a JsValueRef version? The API is specific to debugger and not a common user API which will be used frequently, we can have just one xplat JsValueRef API and remove the Windows wchar_t API.

@jianchun @obastemur @digitalinfinity

[jianchun]

@agarwal-sandeep @jianchun I didn't update JsDiagEvaluateUtf8 to JsDiagEvaluate since we already have one for ChakraFull with same name. As for the JsValueRef expression, what is the suggestion here?

@agarwal-sandeep Is "ChakraDebug.h" another header shared by ChakraCore and ChakraFull, or is it ChakraCore only? Is "JsDiagEvalute" supposed to be a ChakraFull API? (Even if it is, I suspect we haven't and are not releasing it in Windows SDK, thus we are free to make changes to it. Is this correct?)

[obastemur]

Even if it is, I suspect we haven't and are not releasing it in Windows SDK

Makes sense. There is no JsDiagEvalute reference on MSDN.

[agarwal-sandeep]

@jianchun ChakraCore.h is currently only included in ChakraCore, initial plan was to have this in Chakra as well but then the plans changed. Until and unless it goes in Chakra it won't be in Windows SDK. Yes we should be able to make changes but want to confirm with @liminzhu.

[obastemur]

@jianchun @agarwal-sandeep Thanks for the review