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.

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

add docstrings to cwstring and transcode #17218

Merged
merged 1 commit into from
Jul 11, 2016
Merged

Conversation

simonbyrne
Copy link
Contributor

No description provided.

conversion provided by `Cwstring` is if the function is called multiple times with the
same argument.

This is only available on Windows.
Copy link
Member

Choose a reason for hiding this comment

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

Should this mention that this is supposed to be used via Cwstring?

Copy link
Contributor Author

@simonbyrne simonbyrne Jun 30, 2016

Choose a reason for hiding this comment

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

Is it? From what I can tell, the following are equivalent:

ccall(....,Cwstring, str)

or

cwstr = cwstring(str)
ccall(....,Ptr{Cwchar_t}, cwstr)

the advantage of the 2nd one is that if you have to call the function multiple times, you only have to do the conversion once.

Copy link
Member

Choose a reason for hiding this comment

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

I thought Cwstring was exported, but this was not.

Copy link
Member

Choose a reason for hiding this comment

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

Looks like @Keno is right: Cwstring appears in base/exports.jl but not cwstring.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It isn't exported, but is that any reason not to document it?

@kshyatt kshyatt added docs This change adds or pertains to documentation system:windows Affects only Windows strings "Strings!" labels Jul 3, 2016
@simonbyrne simonbyrne force-pushed the sb/string-docstring branch from 6c70f73 to d98b022 Compare July 7, 2016 08:41
@simonbyrne
Copy link
Contributor Author

I realise these functions aren't exported, but probably good to have documented (we may want to export transcode at some point).

The only thing that isn't documented is the recommended way to get a String back from a ccall that returns a UTF-16 string. The examples in Base all use String(Base.transcode(UInt8, x)), where x is a Vector{UInt16}.

@tkelman
Copy link
Contributor

tkelman commented Jul 7, 2016

Can anyone explain the justification for only defining cwstring on Windows?

@StefanKarpinski
Copy link
Member

Can anyone explain the justification for only defining cwstring on Windows?

The definition of cwstring doesn't work on systems where Cwchar_t != UInt16 since we don't currently define transcode methods for UTF-32. I agree that this should be simplified, but that doesn't block this PR.

@StefanKarpinski StefanKarpinski merged commit 0913073 into master Jul 11, 2016
@StefanKarpinski StefanKarpinski deleted the sb/string-docstring branch July 11, 2016 10:49
mfasi pushed a commit to mfasi/julia that referenced this pull request Sep 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation strings "Strings!" system:windows Affects only Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants