Problems with migrating to OWLNext 6.35

[Erwin Lotter]

I could now locate the reason why my OLE server crashed already on startup. It is not related to OLE/OCF but comes from a change in the TApplication C'tor: It throws an exception when the first parameter is 0 as was the default value in previous versions. It seems that tstring does not like the assignment of 0.
When greping the OWL headers I got more than 20 hits on similar changes. Maybe it might be a good idea to use a tstring derived class which accepts a 0 assignment or something similar.

Another issue I found is that the images in Glyphbuttons are shifted to the bottom-right (i.o.w., the top and left borders are some pixels wider than before). Is there a known solution for this?

Erwin

[Vidar Hasfjord]

Hi Erwin,

TApplication C'tor: It throws an exception [null-pointer for tstring]

Thanks for reporting this regression bug. I have registered a ticket [bugs:#355].

When greping the OWL headers I got more than 20 hits on similar changes.

As stated in Strings in OWLNext, the policy for the string overhaul was to preserve well-defined behaviour. I.e. allow null-pointers where they were well-defined before. Hopefully that's not an issue in those 20 other hits, but if you find more breaking cases, let me know, and I will update the article and register tickets.

Glyphbuttons are shifted to the bottom-right

No idea from the top of my head why that is. You may try to identify the revision where the change occurred to get more insight. The log (history) function in the Code section, as well as the Subversion command Blame, may help to identify the revision.

[Erwin Lotter]

Hi Vidar,

Hopefully that's not an issue in those 20 other hits...

I was searching for "tstring& [a-z]+ = tstring()". I didn't check many of the hits, but at least the TRichEdit c'tor was not replaced but paralled by the tstring version.

Glyphbutton images are shifted to the bottom-right

In 6.32.5 the images appear unshifted. When searching the history, I found 3 changes regarding TGlyphbutton: I will look at those.

[Erwin Lotter]

I have now located the modification responsible for the changed appearance of glyphs. When I replace lines 1060-1065 in version 6.35 glyphbtn.cpp:

      TRect srcRect(CelArray->CelRect(index));
      TUIFace face(rect, *CelArray);
      const TPoint dstPt = glyphRect.TopLeft();
      const TUIFace::TState state = TUIFace::Normal;
      const bool pressed = false;
      face.Paint(dc, srcRect, dstPt, state, pressed, false); // no fill (transparent)

by the older code

      CelArray->BitBlt(index, dc, glyphRect.left, glyphRect.top);

the images are centered again. (The effect seems to occur only if the button is to small to let the image fit in the forseen borders.)

[Vidar Hasfjord]

Hi Erwin, good research!

I can now recall the related changes. This code was changed in 6.34 as part of implementing Sebastian's feature request "Theme support for TGlyphButton" [feature-requests:#7]. The original revision [r1785] had code that adjusted the position of the glyph in classic (non-themed) rendering mode. However, this code caused a bug where the glyph would disappear in some circumstances, so the adjustment was removed [r1926]. For the full history of changes to "glyphbtn.cpp" in 6.34, see the Subversion log.

With that in mind, I presume that you see this issue only in classic (non-themed) rendering mode, right? If you find a way to improve the code so that it gives the same glyph adjustment as the old code, without the disappearance issue [r1926], then feel free to submit a patch. That said, I recommend you update your application to enable theme mode.

PS. The Subversion code repository has a lot of information about the history of changes. Since you made no reference to revision numbers etc., I guess you are not fully utilising this information. I recommend taking a closer look at the tools here at this site in the Code section and the trackers for bugs and feature requests, as well as the Subversion features for logging and blaming (very useful for cases like this). I highly recommend TortoiseSVN for working with the code. For example, its "Blame" command gives you a code view in which every line is annotated with the number of its last revision and the author. Just by hovering the mouse cursor over the revision number you get the log message for that revision, explaining why the line was changed. By right-clicking, you can view the full log, run Blame on the previous version, etc.

[Erwin Lotter]

Hi Vidar,

why not switching between old and new code like

if (isThemed) {
  .. actual code
}
else {
  CelArray->BitBlt(index, dc, glyphRect.left, glyphRect.top);
}

This would leave the original behaviour untouched and work fine with themes too, right?

The Subversion code repository has a lot of information ..

You are right, I used the history function only to orientate, then I switched to Winmerge(6.32 vs. 6.35) what I think was the fastest way in this special case (with my limited pre-knowledge).
Nonetheless, I admit that I should shift the launch of using subversion a good deal upwards in my priority list. (In fact, I already started to use a Tortoise-(somehow) package about 15 years ago. But at that time it slowed down the windows explorer so much, that I dropped it.)

[Vidar Hasfjord]

why not switching between old and new code

Looks like an easy and obvious fix. Since you apparently could use an incentive to get Subversion and collaborative development higher on your priority list, I have given you Developer rights to the project! You can now commit code changes to the repository, take ownership of tickets, edit wiki pages, etc. Here is your first exercise:

1. Register a bug ticket regarding this issue, linking to [feature-requests:#7] (the source of the problem).
2. Check out the trunk (version 7) using a Subversion client (e.g. TortoiseSVN).
3. Apply your patch, test it and commit it.
4. Set the status of the ticket to "pending" (awaiting review and release), and add a comment pointing to the revision number of your change (see recent pending/closed tickets for the conventions used).

Then I will review your changes, and if they seem OK, merge them into 6.35 (or likely 6.36).

See Contributing for more about our coding standards and conventions.

[Erwin Lotter]

Check out the trunk (version 7) using a Subversion client (e.g. TortoiseSVN).

How do I get the repositiry URL from https://sourceforge.net/p/owlnext/code/HEAD/tree/trunk ?

[Vidar Hasfjord]

Hi Erwin, kudos to you for taking on my challenge. And welcome as a developer! :-)

For help on checking out the trunk, see Creating a working copy with write-access in the Installation Guide. The guide also has instructions on how to use TortoiseSVN. Let me know if anything is unclear.

[Erwin Lotter]

Ok, I got a working copy now and modified glyphbtn.cpp.
When creating a patch, TortoiseSVN complaines:

Subversion reported an error:

File 'C:\cpp\svn\source\owlcore\glyphbtn.cpp' has inconsistent newlines
Inconsistent line ending style
...

and creates an empty .patch file. I only used the BC5 IDE for editing.
Any idea?

[Vidar Hasfjord]

See Environment Options | Editor | File | Preserve Line Ends. Could it be that this setting affects the EOL style? If not, try opening and saving the file in Notepad WordPad. If that doesn't help, I know Visual Studio will alert you to inconsistent newlines and offer to fix the file.

Edit: This batch command will convert LF to CRLF:

TYPE unix_file | FIND /V "" > dos_file

By the way, you do not need to make a patch file to complete the exercise I outlined. By "apply your patch", I just meant you should make your suggested changes. So if you have made the necessary changes, and you have tested, and you are ready to commit, you can just use the TortoiseSVN command "SVN Commit". That will submit your changes to the code repository.

[Erwin Lotter]

Hi Vidar,

with the Diff tool of TortoiseSVN I could identify linefeeds instead of cr+lf in my changes, but I have no idea where they came from nor could I reproduce them. I corrected this now and tested the changes by building 6.35 and 6.43 with bcc32c 10.1 and VS 2015 respectively. I did not test with a themed app (not avl.).

you do not need to make a patch file

This saves my follow-up question where to put the patch file.

Actually I'm struggling with some new issues which appear when I use VC++ to compile. Most annoying is the use of the TAutoVal(char*) c'tor which works fine with all bc compilers but not with VC++. Do you know whether it is legal to pass a string, e.g. a worksheet name to an Excel automation interface as an TAutoVal(char*)? Or does it work with Borland compilers only by accident?

---

Moderator: Quoted embedded code. Tip: Use backslash or back-quotes to escape formatting control characters, such as "*". See Markdown Syntax. A quick guide is available via the "?" button in the post editor.

[Vidar Hasfjord]

Hi Erwin, congratulations on your first commit to the code repository. I will review and comment in the Developer forum.

Do you know whether it is legal to pass a string, e.g. a worksheet name to an Excel automation interface as an TAutoVal(char*)?

I presume you are here running into the new C++11 standard restrictions on conversions of string literals (see CppReference). The old standard allowed a string literal to be assigned to a char* pointer. C++03 deprecated it, and C++11 then disallowed it. Without a cast, a literal will now only convert to const char*.

:::C++
TAutoVal v("name"); // Error in C++11.
char s[] = "name";
TAutoVal v(s); // Workaround.

[Vidar Hasfjord]

Hi Erwin, I have reviewed your revision [r3593]. If you missed my feedback, see topic [c6b3e137] in the Developers forum.

[Erwin Lotter]

Hi Vidar, I noticed your review. Is some reaction from my side required now?

[Vidar Hasfjord]

See [c6b3e137#9be3].