Skip to content

Tweak to use mxChar in mxarray for String arguments #86

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.

Sign up for GitHub

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

Tweak to use mxChar in mxarray for String arguments #86

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/mxarray.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,13 +406,13 @@ end
# char arrays and string

function mxarray(s::String)
utf16string = transcode(UInt16, s)
utf16string = transcode(mxChar, s)
Copy link
Contributor

@yuyichao yuyichao Nov 8, 2016

Choose a reason for hiding this comment

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

Why? transcode is a julia function and not a MATLAB one so I don't see why this is better.

Copy link
Collaborator Author

@musm musm Nov 8, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

we are marshalling it to matlab using _mx_create_char_array in the next line and matlab uses
http://www.mathworks.com/help/matlab/apiref/mxchar.html

also thread: #76

Copy link
Contributor

@yuyichao yuyichao Nov 8, 2016

Choose a reason for hiding this comment

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

transcode(UInt16, ...) is the API to decode it as UTF16, not transcode(mxChar, ...). I don't see how any of the links are relevant.

Copy link
Collaborator Author

@musm musm Nov 8, 2016

Choose a reason for hiding this comment

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

typealias mxChar UInt16 at the top

Copy link
Contributor

@yuyichao yuyichao Nov 8, 2016

Choose a reason for hiding this comment

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

Right.

  1. They are the same so why change it?
  2. You are calling julia API defined on UInt16 and documented as so and why confuse the reader about what you are calling using an matlab type alias?

Copy link
Collaborator Author

@musm musm Nov 8, 2016

Choose a reason for hiding this comment

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

Ok if you feel that way close this and let's move on. I just want to move on and not create a big issue. I just felt that this was more consistent since it's used below https://github.com/musm/MATLAB.jl/blob/6993d1e50f38ae73ec45a0bf7a4bbe64ac4058ca/src/mxarray.jl#L646

to convert the String back to Julia.

pm = ccall(_mx_create_char_array, Ptr{Void}, (mwSize, Ptr{mwSize},), 2,
_dims_to_mwSize((1, length(utf16string))))
mx = MxArray(pm)
ccall(:memcpy, Ptr{Void}, (Ptr{Void}, Ptr{Void}, UInt), data_ptr(mx), utf16string,
length(utf16string)*sizeof(UInt16))
mx
length(utf16string)*sizeof(mxChar))
return mx
end

# cell arrays
Expand Down