On January 2, 2003 10:12 pm, Alexandre Julliard wrote:
Added wine_dbg_sprintf function that allocates a temporary bufferin the per-thread strings area.
Cool. But what about functions that need to build the string. For example, look at debuglvitem_t() in listview.c. Can we get somehow to the max_size?
Added inline functions to format POINT, SIZE and RECT structures.
Can we pretty please get shorter names for them in Wine? Like without the wine_ prefix? :)
Dimitrie O. Paun wrote:
Added inline functions to format POINT, SIZE and RECT structures.Can we pretty please get shorter names for them in Wine? Like without the wine_ prefix? :)
I kind of like the idea of usaing a uniform prefix of wine_ for all symbols that are unique to wine ... - Dan
On January 2, 2003 10:55 pm, Dan Kegel wrote:
I kind of like the idea of usaing a uniform prefix of wine_ for all symbols that are unique to wine ...
We do externally, I am referring to internal usage only. And for internal stuff, we almost never use wine_ as a prefix.
"Dimitrie O. Paun" dpaun@rogers.com writes:
On January 2, 2003 10:12 pm, Alexandre Julliard wrote:
Added wine_dbg_sprintf function that allocates a temporary bufferin the per-thread strings area.
Cool. But what about functions that need to build the string. For example, look at debuglvitem_t() in listview.c. Can we get somehow to the max_size?
I'd rather not let users write to the buffer directly. IMO a function like debuglvitem_t should be dumping things directly to the output, not build a huge string and then print it. The temporary strings buffer should be only for really short lived strings.
Added inline functions to format POINT, SIZE and RECT structures.Can we pretty please get shorter names for them in Wine? Like without the wine_ prefix? :)
I'm not sure we want that. I know it's what we have been doing so far, but it's very confusing to have to use different debugging functions depending on what part of the code you are in. I think we should try to move towards a more unified interface.
On January 2, 2003 11:22 pm, Alexandre Julliard wrote:
I'd rather not let users write to the buffer directly. IMO a function like debuglvitem_t should be dumping things directly to the output, not build a huge string and then print it. The temporary strings buffer should be only for really short lived strings.
Well, I've tried the dump-directly-to-output version, and it invariably leads to ugly code. The usage of debuglvitem_t is _so_ much nicer compared to anything else. And uniform. And easy to understand. Granted, the function is a bit big, but the resulting string is not that 'huge'. In fact, the function is big because I'm trying to minimize the string (and thus uncluttered the output). The max_size of 200 that we have now is more than enough for anything we need. As for the lifetime of the string, it's _identical_ to the RECT, POINT, and SIZE ones.
Added inline functions to format POINT, SIZE and RECTstructures.
Can we pretty please get shorter names for them in Wine? Like without the wine_ prefix? :)
I'm not sure we want that. I know it's what we have been doing so far, but it's very confusing to have to use different debugging functions depending on what part of the code you are in. I think we should try to move towards a more unified interface.
It depends on how you look at it. We're going to end up with a mixture of wine_ and non-wine_ stuff that is a lot more confusing and ugly than what we have now.
On the other hand, cluttering all on Wine's code with wine_* and WINE_TRACE, and WINE_ERR, is not also very appealing either. Look, let's not try to solve world's problems: this is not the debugging API to end all debugging APIs. It's Wine's internal one. And it's present interface is _nice_, and easy to use. And this is paramount for a good API. I think it would be a mistake to drop all that for a dubious generality. Any program using it is going to do something of the form: #define DEBUG WINE_TRACE Nobody is going to use the long symbols as they are anyway. In fact, I suggest that instead of the __WINE__ test for defining the short API we should test for WINE_DBG_SHORT_API so that others can use it as well. 'Cause it's nice. And this way we can use the short API in all of our own Winelib apps so there's no confusion anymore, _and_ we can stick to the short API.
"Dimitrie O. Paun" dpaun@rogers.com writes:
Well, I've tried the dump-directly-to-output version, and it invariably leads to ugly code. The usage of debuglvitem_t is _so_ much nicer compared to anything else. And uniform. And easy to understand. Granted, the function is a bit big, but the resulting string is not that 'huge'. In fact, the function is big because I'm trying to minimize the string (and thus uncluttered the output). The max_size of 200 that we have now is more than enough for anything we need. As for the lifetime of the string, it's _identical_ to the RECT, POINT, and SIZE ones.
Not really; it's doing a lot more stuff between the time the string would be allocated and the time it is printed, even including other buffer allocations which would make it impossible to release the unneeded string space. If you don't want to make it dump directly to the output, you could put things into a stack buffer and then do a wine_dbg_sprintf to copy that buffer into the strings space.
On the other hand, cluttering all on Wine's code with wine_* and WINE_TRACE, and WINE_ERR, is not also very appealing either. Look, let's not try to solve world's problems: this is not the debugging API to end all debugging APIs. It's Wine's internal one. And it's present interface is _nice_, and easy to use.
The thing is that it's not only internal, it's useful in Winelib apps because there are no good debugging functions in the Windows API. And everybody I know who has tried to use the debug API in a Winelib app has been confused by the different names.
Now I'm not saying we should rename TRACE everywhere, but for new functions, especially ones that aren't used all that much, I don't think an extra 'wine' prefix is a problem, and it makes things a lot easier to follow.
On January 3, 2003 12:45 pm, Alexandre Julliard wrote:
The thing is that it's not only internal, it's useful in Winelib apps because there are no good debugging functions in the Windows API. And everybody I know who has tried to use the debug API in a Winelib app has been confused by the different names.
That includes me. But since we're not going to rename everything (and this makes me so happy, I would have hate it if we did), the confusion is still going to be there. People are still going to have to learn that there is this mapping. But than, for magically chosen, obscure functions, they will have to learn another rule!
Thing is, inconsistency always hurts more, even if done toward a better goal. How can things be any easier in such a situation? Furthermore, in this very case it's not clear at all to me we are aiming at anything better. I have seen no project that uses this long names for its debugging API. There are many reasons why not, I will not hash them here, but this simple observation should tell us something.
I stand by my suggestion of having a way of exporting the shorter API independently of defining __WINE__. 99% of apps will be able to use it as is, and they'll be a lot happier. Our own internal apps will certainly be able to, and that's going to eliminate all confusion. For the ones that can't use it, most likely they'll go through the exercise of defining a shorter API anyhow, so nothing is lost or made harder for them.
"Dimitrie O. Paun" dpaun@rogers.com writes:
That includes me. But since we're not going to rename everything (and this makes me so happy, I would have hate it if we did), the confusion is still going to be there. People are still going to have to learn that there is this mapping. But than, for magically chosen, obscure functions, they will have to learn another rule!
That's not another rule, it's just the standard behavior: functions are called the same way under Wine and under Winelib. If we use the wine_* functions in Wine too, then the rule is that in Winelib everything is identical, except that TRACE/ERR/WARN/FIXME have to be prefixed with WINE_. That's certainly easier and more consistent than having a separate rule for the debugstr functions that is different from everything else.
Furthermore, in this very case it's not clear at all to me we are aiming at anything better. I have seen no project that uses this long names for its debugging API. There are many reasons why not, I will not hash them here, but this simple observation should tell us something.
We can certainly come up with shorter names if that's what bothers you; personally I don't think that e.g. wine_dbgstr_guid is much worse than debugstr_guid, but I'm certainly open to other suggestions. The important thing is to have only one name and stick to it everywhere; this in turn implies that it must be unique enough to not risk conflicting with other identifiers.
On January 3, 2003 01:54 pm, Alexandre Julliard wrote:
That's not another rule, it's just the standard behavior: functions are called the same way under Wine and under Winelib. If we use the wine_* functions in Wine too, then the rule is that in Winelib everything is identical, except that TRACE/ERR/WARN/FIXME have to be prefixed with WINE_. That's certainly easier and more consistent than having a separate rule for the debugstr functions that is different from everything else.
Granted. But there is a fundamental difference: all other wine_* functions are so esoteric and rarely used, that we could even call them wine_esoteric_function_... and no one would be bothered by it :) Besides, they are meant to be called by some sort of portability layer in any decent app anyhow, so you'll have a few instances of those in any app, no matter how large. The debug functions are on the other hand the _most_ used functions in any app. A lot more than even the string functions. And those have names that are 6-8 char long. wine_dbgstr_point is 17 chars long. And in a TRACE statement, you're not unlikely to use 2-3 of these:
WINE_TRACE("(lprc=%s, lppt=%s, nSize=%d)\n", wine_dbgstr_rect(lprc), wine_dbgstr_point(lppt), nSize);
There is so many stuff in there... it's way too verbose, you don't know what's going on there anymore. Contrast with this:
TRACE("(lprc=%s, lppt=%s, nSize=%d)\n", dbgstr_rect(lprc), dbgstr_point(lppt), nSize);
or even:
TRACE("(lprc=%s, lppt=%s, nSize=%d)\n", dbg_rect(lprc), dbg_point(lppt), nSize);
Obviously, if people are going to use our interface, they are not going to use our long names directly. Why not allow them the option of using our short names? How many apps are going to have a TRACE macro _if_ they decide to use our debugging API? And if they do, they can -DWINE_NO_SHORT_DBG_API and be done with it (or we can enable it only if -DWINE_SHORT_DBG_API).
"Dimitrie O. Paun" dpaun@rogers.com writes:
Granted. But there is a fundamental difference: all other wine_* functions are so esoteric and rarely used, that we could even call them wine_esoteric_function_... and no one would be bothered by it :)
It's not only the wine_* functions. There is no conceptual difference between code that resides in a dll inside the Wine tree and code in a Winelib dll outside the Wine tree. You can happily copy code back and forth between Wine and the Winelib app, and you expect it to work. That's the principle of least suprise again <g>
Obviously, if people are going to use our interface, they are not going to use our long names directly. Why not allow them the option of using our short names? How many apps are going to have a TRACE macro _if_ they decide to use our debugging API? And if they do, they can -DWINE_NO_SHORT_DBG_API and be done with it (or we can enable it only if -DWINE_SHORT_DBG_API).
I'm not opposed to a define to export the TRACE/FIXME/ERR/WARN macros, but that's not a general solution. We don't want to have to explain "sure the API is broken but if you define this magic symbol you can make it sane again". We should define a sane API in the first place.
On January 3, 2003 02:37 pm, Alexandre Julliard wrote:
You can happily copy code back and forth between Wine and the Winelib app, and you expect it to work. That's the principle of least suprise again <g>
Darn, I knew you gonna get me on this one! :) Hehe, what goes around comes around. ;)
I'm not opposed to a define to export the TRACE/FIXME/ERR/WARN macros, but that's not a general solution. We don't want to have to explain "sure the API is broken but if you define this magic symbol you can make it sane again". We should define a sane API in the first place.
Hey, it is the same API afterall. It is not broken, it just uses different names, and you have to admit the names are a bit nicer on the eye. And speaking of these prefixes and all, I don't see why we have to use wine_ to prefix things. We can imagine this is a debugging API that Wine uses as well, so it might as well use a different prefix. What about dbg_? :)
If you really want to export only one symbol, fine, lets export the short ones only. The 1% of apps that have conflicts can do things (in mydebug.h):
#define TRACE DEBUG #define WARN WARNING #include <wine/debug.h>
They'll most like have to do something like this anyway, with the only difference that the defines come after the include, but hey, it's a small price to pay! And if they don't like it, they don't need to use our headers at all, they can implement their own macros on top of the functions we export from ntdll. Come on, they are a few lines of code, is it really worth complicating the Wine code for it?
"Dimitrie O. Paun" dpaun@rogers.com writes:
They'll most like have to do something like this anyway, with the only difference that the defines come after the include, but hey, it's a small price to pay! And if they don't like it, they don't need to use our headers at all, they can implement their own macros on top of the functions we export from ntdll. Come on, they are a few lines of code, is it really worth complicating the Wine code for it?
Yes; if we make that API available then we have a responsibility to try to make it usable for as many situations as possible, even if that means some extra work for us. We can't simply use what's most convenient for us and ignore the problems it will cause for others; if we do that then we might as well not export the API at all.
So we need to find a compromise between convenience and uniqueness. Clearly the TRACE etc. macros are not unique enough, we already have had several conflicts with them, that's why we have the WINE_* form. And for the other functions we need some kind of prefix too; not necessarily "wine_dbgstr_" but I don't think "dbg_" is unique enough.
On Fri, 3 Jan 2003, Dimitrie O. Paun wrote: [...]
Obviously, if people are going to use our interface, they are not going to use our long names directly. Why not allow them the option of using our short names? How many apps are going to have a TRACE macro _if_ they decide to use our debugging API?
In the MFC, afx.h defines a TRACE macro. So no MFC-based application can use Wine's short names. Actually, no MFC-based application will compile correctly if we export our own TRACE macro (having macro redefinitions is not acceptable).
To me that's a pretty significant section of potential Winelib apps (both in importance and in numbers). Many commercial applications are using Visual C++ and Winelib and I think it's important to make it easy for them to use Winelib to recompile their applications on Unix.
So I think TRACE should not be exported by default. I am not opposed to a -DWINE_SHORT_DBG_API however.
I'm not too concerned about the length of these function names but we could reduce the name length by replacing the 'WINE_' prefix by just 'W' though that would slightly increase the risk of collisions. And we could replace 'debugstr_' with just 'debug_' or 'dbg_' or maybe 'dump_' (though dump is not quite correct).
So the main choices are: WINE_TRACE WTRACE
and
wine_debugstr_rect wine_debug_rect wine_dump_rect wine_dbg_rect wdump_rect wdbg_rect wstr_rect
Dimitrie O. Paun wrote:
WINE_TRACE("(lprc=%s, lppt=%s, nSize=%d)\n", wine_dbgstr_rect(lprc), wine_dbgstr_point(lppt), nSize);
There is so many stuff in there... it's way too verbose, you don't know what's going on there anymore. Contrast with this:
TRACE("(lprc=%s, lppt=%s, nSize=%d)\n", dbgstr_rect(lprc), dbgstr_point(lppt), nSize);
Yes, but consider the poor sod who has already used the identifier TRACE (precisely because it's a convenient name, as you point out) and is trying to build with winelib. Sure, we could have some switch to turn on and off the short names, but that gets hard to read...
- Dan
On January 3, 2003 06:48 pm, Dan Kegel wrote:
Yes, but consider the poor sod who has already used the identifier TRACE (precisely because it's a convenient name, as you point out) and is trying to build with winelib. Sure, we could have some switch to turn on and off the short names, but that gets hard to read...
I don't understand -- no winelib app include wine/debug.h, none will get this problem. Unless they _explicitely_ decide to use our debugging API, and they _explicitly_ #include <wine/debug.h>. In which case they are aware of the problem. And in any event, they will want their own debug.h header where they can do whatever renaming they want. In fact, if we export just the long versions (which I'm OK with), they _will_ have to have their own debug.h to provide more convenient names, in which case they will probably be different from what we use in Wine anyway. Or we can suggest our short names, so their debug.h can look like so:
#define WINE_NICE_DBG_API #include <wine/debug.h>
Of course, MFC apps can do a number of trivail things to avoid the TRACE conflict. So they are not a problem in any event.
It's simple, nice, easy to use and understand. In the long run we end up with a lot more uniformity.
But this is besides the point. What I'm arguing against is using the long names internally. This makes little sense, since I am virtually sure nobody else will. So we will loose clarity, neatness, and convenience for the very purpose of uniformity with some Winelib apps, that's not going to be there in the first place since they are not gonna use the long names, and are going to invent their own anyhow.
What I'm saying is that if we provide people with an easy way to have convenient names, 99.99% will simply use those names, and we'll have a lot more of this uniformity. The few that choose not to, have a quite a few easy ways to choose their own names.
Dimitrie O. Paun wrote:
But this is besides the point. What I'm arguing against is using the long names internally. This makes little sense, since I am virtually sure nobody else will. So we will loose clarity, neatness, and convenience for the very purpose of uniformity with some Winelib apps, that's not going to be there in the first place since they are not gonna use the long names, and are going to invent their own anyhow.
Hmm. I don't see a problem with WINE_TRACE myself. It seems short enough, and the source code gets read a lot more often than it gets written, so the extra clarity is good. - Dan
On January 3, 2003 07:08 pm, Dan Kegel wrote:
Hmm. I don't see a problem with WINE_TRACE myself. It seems short enough, and the source code gets read a lot more often than it gets written, so the extra clarity is good.
Maybe I'm in my I_just_don't_get_it_mode, but I really don't! What clarity? Why is it important that the debug API is a Wine thing? Why not look at it as a 3rd party debugging API? Would you still call it WINE_TRACE? No, of course not, it would be silly. Why expose this implementation detail (that this is really implemented inside of Wine) in the interface? What possible benefit do you get out of it? Quite the contrary, it will just obfuscate the source. People think that very_long_identifiers_for_everything are a Good Thing. They are not. What about: WINE_SUPPORT_DEBUG_API_TRACE? Do we gain anything from knowing all that? I don't think so.
But we don't seem to reach a consensus on this one. So I will state this for the record: -- I think using WINE_TRACE instead of TRACE inside of Wine makes little sense, and it will uglify everything. I strongly oppose it. Making the sources ugly is something we should not do lightly. People work on Wine for pleasure, and that definitely diminishes it. Making the most common thing more tedious falls into the same category. -- Let's think of the debugging API on it's own terms, not as part of wine. In fact, wine_ should not be part of the name at all. I suggested dbg_, even though Alexandre is right that it might cause problems. I'd like to remark that debugging APIs are special in that they are used orders of magnitude more often than any other API. In all natural languages, very common words often form exceptions to the rules of the language for shortness sake. Witness the verb "to be" in most languages. For this reasons, I don't think it's a mistake to treat it a bit differently. I suggest that we export symbols with the dbg_ prefix by default, but we allow an easy way for the user to change that, maybe like so: #define WINE_DEBUG_API_PREFIX(s) d_##s #include <wine/debug.h>
Bottom line, I don't really care what we export. I do care what we use internally. A great deal. In fact, if we export long symbols, I don't think it matters how long they are, people will just invent short synonyms. But why can't we invent short synonyms for Wine itself? :)
Dimitrie O. Paun wrote:
Bottom line, I don't really care what we export. I do care what we use internally. A great deal. In fact, if we export long symbols, I don't think it matters how long they are, people will just invent short synonyms. But why can't we invent short synonyms for Wine itself? :)
I'd argue that for clarity's sake, we should discourage people from concocting short synonyms for WINE_TRACE... so if we pick something too long for Dimitrie, we're in for a heap of arguments.
How about _WTRACE? It gets zero hits on google...
Dimitrie O. Paun wrote:
Hmm. I don't see a problem with WINE_TRACE myself. It seems short enough, and the source code gets read a lot more often than it gets written, so the extra clarity is good.
Maybe I'm in my I_just_don't_get_it_mode, but I really don't! What clarity? Why is it important that the debug API is a Wine thing? Why not look at it as a 3rd party debugging API? Would you still call it WINE_TRACE? No, of course not, it would be silly. Why expose this implementation detail (that this is really implemented inside of Wine) in the interface? What possible benefit do you get out of it? Quite the contrary, it will just obfuscate the source. People think that very_long_identifiers_for_everything are a Good Thing. They are not. What about: WINE_SUPPORT_DEBUG_API_TRACE? Do we gain anything from knowing all that? I don't think so.
It doesn't have to be long, it just has to be unique, and have a prefix that implies that it comes from Wine. That way nobody is likely to be confused about whose TRACE macro it is. It's important when visually scanning source code that it be fairly obvious what package each identifier comes from, IMHO.
_W might be enough of a unique and common prefix... - Dan
On January 3, 2003 07:48 pm, Dan Kegel wrote:
It doesn't have to be long, it just has to be unique, and have a prefix that implies that it comes from Wine. That way nobody is likely to be confused about whose TRACE macro it is. It's important when visually scanning source code that it be fairly obvious what package each identifier comes from, IMHO.
And it's even more important that visually scanning source code does not hurt the eye. For me, it's a sine qua non condition -- I simply do not work (unless paid, of course :)) on ugly stuff. There is no reason to support apps that do _WTRACE, _TTRACE, _XTRACE, etc. There is also no reason to show at source level that the trace statement from Wine is used. None. This is like having .sh, or .py extensions to scripts. They just expose implementation details that are better kept hidden.
So no, _WTRACE is a lot worse than WINE_TRACE since it's so ugly. Dan, don't tell me you want people to use that as is :)
-
Alexandre Julliard -
Dan Kegel -
Dimitrie O. Paun -
Francois Gouget