shotgunsoftware / tk-multi-importcut

Import Cut app
Other
0 stars 0 forks source link

37717 unicode support #54

Closed sgsteph closed 8 years ago

sgsteph commented 8 years ago
raphaelmatto commented 8 years ago

Looks good, Stéphane! My main comment is the new uvariable names are a little hard to read. Maybe switch it to vars like:

u_path u_value u_new u_old

... instead of:

upath uvalue unew uold

Besides that, I wanted to make sure you checked the Omit statuses. I.e., if unicode characters are set as sg_status_list statuses on Shots, will they play nice with our pull-down menus? (It looks like you did some work to make sure the notification Groups work).

manneohrstrom commented 8 years ago

Python hungarian notation lol! >.<

@thebeeland mentioned today that by emitting strings as objects instead of as strings, their stringiness is preserved. Maybe something to try?

sgsteph commented 8 years ago

@manneohrstrom I think that emitting strings as objects would imply keeping a reference to them around ? By using regular unicode, Qt converts them internally to QString so it is safe to just emit the string and forget about it, which I think wouldn't be the case with PyObject. It has the benefit to be consistent as well: any string coming from Qt is unicode (UI, signal ) and must be converted, so a general rule can be applied.

raphaelmatto commented 8 years ago

Looks good, no comments, merge at will.