Re: [lmi] trivial patch: don't use deprecated GetPrintableName()

[Greg Chicares]

On 2008-05-10 20:30Z, Vadim Zeitlin wrote:
> 
> --- old/view_ex.cpp     2008-03-25 12:54:28 +0000
> +++ new/view_ex.cpp     2008-05-10 15:32:22 +0000
> @@ -178,10 +178,8 @@
> 
>  std::string ViewEx::base_filename() const
>  {
> -    wxString z;
> -    GetDocument()->GetPrintableName(z);
> -    std::string t(z);
> -    if(0 == t.size())
> +    std::string t(GetDocument()->GetUserReadableName());
> +    if(t.empty())
>          {
>          t = "Anonymous";
>          }
> 
> --- old/illustration_view.cpp   2008-03-27 16:08:16 +0000
> +++ new/illustration_view.cpp   2008-05-10 20:17:22 +0000
> @@ -341,9 +341,7 @@
>  {
>      LMI_ASSERT(ledger_values_.get());
> 
> -    wxString z;
> -    document().GetPrintableName(z);
> -    std::string filename(z.c_str());
> +    std::string filename(document().GetUserReadableName());
> 
>      std::string pdf_out_file = write_ledger_as_pdf(*ledger_values_, 
> filename);
>      file_command()(pdf_out_file, action);

Shouldn't we add 'c_str()', e.g.:
  std::string S(some_document.GetUserReadableName().c_str());
? Paraphrasing an off-list email you sent me 2008-01-29T14:24Z,
this is always okay:
  std::string S("STD");
  wxString W(S);
but c_str() is preferred here:
  wxString W("WX");
  std::string S(W.c_str());
or else we're restricted to a wxUSE_STL build--and, IIRC, that
was an issue for you because you maintain so many builds.

But maybe I misunderstood: the original code was inconsistent
in this respect, and I don't recall it being identified as a
problem.

And I certainly agree that empty() is better than 0==size(),
but doesn't the revised documentation:

  
http://www.lpthe.jussieu.fr/~zeitlin/wxWidgets/docs/wxwin_wxdocument.html#wxdocumentgetuserreadablentablename
| The default implementation uses the document title, if any,
| of the name part of the document filename if it was set or,
| otherwise, the string unnamed.

guarantee that a nonempty string will be returned, so that we
can eliminate that clause?

>  And I also should have probably mentioned in my previous email that I'm
> going to submit a slightly less trivial patch fixing a similar problem with
> MakeDefaultName() very soon so if you prefer to apply both of them at once
> you should wait for it.

I was about to ask whether you had a replacement for that
function, too. Aside from that, the only other place where lmi
seems to default-construct a local wxString in order to pass it
by non-const reference to a library function is with
  wxFileType::GetOpenCommand()
  wxFileType::GetPrintCommand()
where I guess there really is a reason for returning bool and
passing the string by reference, so that doesn't trouble me.