May 2012 - Wine-Devel - list.winehq.org

Re: kernel32: Add UTF-7 support

by Ben Klein

On 8 May 2012 12:50, Alex Henrie <alexhenrie24(a)gmail.com> wrote: > 2012/5/7 Ben Klein <shacklein(a)gmail.com>: >> Without being a regular contributor, here are a few tips from a cursory glance: >> 1) Whitespace change on at least one line > > The only change to whitespace I made was: > > - if( flags) > + if (flags) > > I don't think this is a big deal. Sure, but you might as well fix it for the next submission. >> 2) Why the change from ERROR_INVALID_PARAMETER to ERROR_INVALID_FLAGS? > > To match the behavior of Windows. I verified this with both MSDN and > empirical tests. If the flags parameter is not null, all versions of > Windows tested report ERROR_INVALID_FLAGS with the possible exception > of Windows NT4. > See http://testbot.winehq.org/JobDetails.pl?Key=18148 I see. In that case, this should go in a separate patch as it's not specific to UTF7 support. >> 3) New function defs do not match format of surrounding defs in unicode.h > > This was a hard one. I didn't know if I should keep Katayama's use of > INT or change the functions to use int like the UTF-8 functions do. So > you're saying it should all be int? Do LPCSTR and LPCWSTR need to be > changed to something else as well? I'm GUESSING that it all needs to be changed to match the surrounding definitions. >> 4) No new tests to confirm the behaviour is correct (or mention of >> current tests succeeding) > > dlls/kernel32/tests/locale.c already has just as many UTF-7 tests as > it has UTF-8 tests. What tests do we need for UTF-7 which are not > needed for UTF-8? > > Thanks for your quick response, > > -Alex > >

May 8, 2012

Re: kernel32: Add UTF-7 support

by Alex Henrie

2012/5/7 Ben Klein <shacklein(a)gmail.com>: > Without being a regular contributor, here are a few tips from a cursory glance: > 1) Whitespace change on at least one line The only change to whitespace I made was: - if( flags) + if (flags) I don't think this is a big deal. > 2) Why the change from ERROR_INVALID_PARAMETER to ERROR_INVALID_FLAGS? To match the behavior of Windows. I verified this with both MSDN and empirical tests. If the flags parameter is not null, all versions of Windows tested report ERROR_INVALID_FLAGS with the possible exception of Windows NT4. See http://testbot.winehq.org/JobDetails.pl?Key=18148 > 3) New function defs do not match format of surrounding defs in unicode.h This was a hard one. I didn't know if I should keep Katayama's use of INT or change the functions to use int like the UTF-8 functions do. So you're saying it should all be int? Do LPCSTR and LPCWSTR need to be changed to something else as well? > 4) No new tests to confirm the behaviour is correct (or mention of > current tests succeeding) dlls/kernel32/tests/locale.c already has just as many UTF-7 tests as it has UTF-8 tests. What tests do we need for UTF-7 which are not needed for UTF-8? Thanks for your quick response, -Alex

May 8, 2012

Re: kernel32: Add UTF-7 support

by Ben Klein

On 8 May 2012 11:21, Alex Henrie <alexhenrie24(a)gmail.com> wrote: > Hi all, > > Could I get some feedback on > http://source.winehq.org/patches/data/85898 ? It was marked "Rejected" > a week ago without explanation. Without being a regular contributor, here are a few tips from a cursory glance: 1) Whitespace change on at least one line 2) Why the change from ERROR_INVALID_PARAMETER to ERROR_INVALID_FLAGS? 3) New function defs do not match format of surrounding defs in unicode.h 4) No new tests to confirm the behaviour is correct (or mention of current tests succeeding) > Most of the code was authored by Katayama Hirofumi, whose patches have > been accepted into Wine before. He gave permission to use his code > under the LGPL at http://bugs.winehq.org/show_bug.cgi?id=27388 > > What more needs to be done before this patch can be accepted? > > -Alex > >

May 8, 2012

kernel32: Add UTF-7 support

by Alex Henrie

Hi all, Could I get some feedback on http://source.winehq.org/patches/data/85898 ? It was marked "Rejected" a week ago without explanation. Most of the code was authored by Katayama Hirofumi, whose patches have been accepted into Wine before. He gave permission to use his code under the LGPL at http://bugs.winehq.org/show_bug.cgi?id=27388 What more needs to be done before this patch can be accepted? -Alex

May 8, 2012

Re: msi: Tweak msiexec's usage message.

by Francois Gouget

On Sat, 5 May 2012, Frédéric Delanoy wrote: [...] > The msiexec conventions look sane to me: [ ] for optional and { } for > mandatory parameters (*only* when a choice/list is involved). It's > also what's used in other wine commands "/?" results (e.g. 'start', > 'ipconfig', 'regedit', etc.). It's no so consistent. msiexec uses names like 'product_code' but taskkill uses 'ProcessID' (which aspell does not like) and wineconsole uses '<command>'. regedit describes what 'filename' and 'regpath' means independently of the option that uses these, msiexec and start do not describe what things like 'package' or 'program_filename' are. In xcopy it's 'Usage: ... Where: ...' while in wineconsole it's 'Usage: ... Options: ...' and msiexec has a totally different structure (and does not very clearly differentiate commands and options). For ipconfig there's spaces around brackets while in dxdiag or msiexec where's no space around the brackets. xcopy says '[/Y] [/-Y]' when it should probably be '[/Y | /-Y]', although msiexec would likely describe outside of usage line as '/{Y|-Y}'. -- Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ Advice is what we ask for when we already know the answer but wish we didn't -- Eric Jong

May 7, 2012

Re: [Programs] Notepad: sync with ReactOS notepad (2)

by carlo.bramix＠libero.it

Hello friends, >> Because you do seriously believe that ReactOS developers reversed the >> Windows notepad?! Along with Solitaire & calculator, I suppose? >> Common sense is a powerful tool, let's make use of it :-). > >It's a matter of trust. If a person is known to use illegal practices >to write ReactOS code then even adding one line of code to notepad >makes her/his contribution dangerous for the entity accepting that code. > >> For the record, ReactOS as well disallows any person who has seen the >> Microsoft Windows code to comment, contribute, commit to the project. > >That's just a blanket statement, nothing more. It's hard to believe >to it if the person(s) publicly accused in using leaked code still >commit to ReactOS without any limitation. as I already wrote, I totally agree. Please reject the patches. Sincerely, Carlo Bramini.

May 7, 2012

Re: [Programs] Notepad: sync with ReactOS notepad (2)

by Dmitry Timoshkov

Pierre Schweitzer <pierre(a)reactos.org> wrote: > Because you do seriously believe that ReactOS developers reversed the > Windows notepad?! Along with Solitaire & calculator, I suppose? > Common sense is a powerful tool, let's make use of it :-). It's a matter of trust. If a person is known to use illegal practices to write ReactOS code then even adding one line of code to notepad makes her/his contribution dangerous for the entity accepting that code. > For the record, ReactOS as well disallows any person who has seen the > Microsoft Windows code to comment, contribute, commit to the project. That's just a blanket statement, nothing more. It's hard to believe to it if the person(s) publicly accused in using leaked code still commit to ReactOS without any limitation. -- Dmitry.

May 7, 2012

Re: [Programs] Notepad: sync with ReactOS notepad (2)

by Pierre Schweitzer

Because you do seriously believe that ReactOS developers reversed the Windows notepad?! Along with Solitaire & calculator, I suppose? Common sense is a powerful tool, let's make use of it :-). For the record, ReactOS as well disallows any person who has seen the Microsoft Windows code to comment, contribute, commit to the project. Once again, common sense... Regarding the authors of the notepad patches/commits, you have all of them with the SVN logs (even when it's an applied patch). Regars, Pierre S. Le dimanche 06 mai 2012 à 21:44 +0200, carlo.bramix(a)libero.it a écrit : > Hello friends, > > >The issue is whether the appropriate guidelines were followed > >(http://wiki.winehq.org/CleanRoomGuidelines) One issue is that Wine > >does not allow people that have ever viewed Microsoft source code to > >contribute to Wine, but there are other issues. You'll have to talk > >to Alexandre and see how he feels about this specific issue, but if > >you've followed all of the guidelines and contribute only your own > >changes then he might be ok with your contribution. > > I see. > Perhaps it would be better to reject all these patches since I'm not the only > author of all the changes. > I understand that this is done to protect yourself and the project, because > you do not know the people who touched the sources of the notepad, who they are > and how they collected their information, so I agree with you. > > Sincerely, > > Carlo Bramini. > > >

May 6, 2012

Re: [Programs] Notepad: sync with ReactOS notepad (2)

by carlo.bramix＠libero.it

Hello friends, >The issue is whether the appropriate guidelines were followed >(http://wiki.winehq.org/CleanRoomGuidelines) One issue is that Wine >does not allow people that have ever viewed Microsoft source code to >contribute to Wine, but there are other issues. You'll have to talk >to Alexandre and see how he feels about this specific issue, but if >you've followed all of the guidelines and contribute only your own >changes then he might be ok with your contribution. I see. Perhaps it would be better to reject all these patches since I'm not the only author of all the changes. I understand that this is done to protect yourself and the project, because you do not know the people who touched the sources of the notepad, who they are and how they collected their information, so I agree with you. Sincerely, Carlo Bramini.

May 6, 2012

Re: [Programs] Notepad: sync with ReactOS notepad (2)

by Erich E. Hoover

On Sun, May 6, 2012 at 7:49 AM, carlo.bramix(a)libero.it <carlo.bramix(a)libero.it> wrote: > > thank you for working on this. I think you will improve the chances for > > your patches to be accepted if you give them individually meaningful > > names, like > > ... > > Actually, I followed the tip I read into the wiki of WINE in "Patch submission > guidelines", so I tried to imitate the style of other patches for WINE and for > this reason I used an identical title with a progressive number as > subject. > ... What that instruction is meant to say is that patches that are part of a series (meant to go one after the other) should be numbered sequentially, but still have a descriptive title. So, something like: [PATCH 1/3] notepad.exe: Fix formatting of resource file. [PATCH 2/3] notepad.exe: Do not use the MSVC reserved keyword "class". [PATCH 3/3] notepad.exe: Unify find and replace functionality. Note that these examples are just from scanning your descriptions, there may be better patch titles that you can use. > ... > Well, ReactOS notepad had been imported from WINE long time ago and various > people did some changes on it and I'm included in them since, in this specific > case, the status bar implementation had been almost rewritten by me. > ... The issue is whether the appropriate guidelines were followed (http://wiki.winehq.org/CleanRoomGuidelines) One issue is that Wine does not allow people that have ever viewed Microsoft source code to contribute to Wine, but there are other issues. You'll have to talk to Alexandre and see how he feels about this specific issue, but if you've followed all of the guidelines and contribute only your own changes then he might be ok with your contribution. Erich Hoover ehoover(a)mines.edu

May 6, 2012