Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(184)

Issue 347070043: Improve relocation debug messages.

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 months, 2 weeks ago by lemzwerg
Modified:
2 months, 1 week ago
Reviewers:
dak, dan
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Improve relocation debug messages. This patch contains the following commits. * relocate.cc: Don't use double forward slashes in file names. * main.cc, relocate.cc: Improve relocation debug messages. This commit does the following. * Add `_f ()' where necessary. * Improve readability of code by omitting some `.c_str ()' conversions. * Synchronize formatting of messages. * Improve messages to make the more readable. Also add some new ones. * Indent all messages related to relocation. * relocate.cc: Add `indent' parameter to `sane_putenv'. We are going to use that to provide better formatting of debug messages.

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+67 lines, -38 lines) Patch
M lily/include/relocate.hh View 1 chunk +4 lines, -1 line 0 comments Download
M lily/main.cc View 2 chunks +4 lines, -1 line 0 comments Download
M lily/relocate.cc View 16 chunks +59 lines, -36 lines 1 comment Download

Messages

Total messages: 7
dak
https://codereview.appspot.com/347070043/diff/1/lily/relocate.cc File lily/relocate.cc (right): https://codereview.appspot.com/347070043/diff/1/lily/relocate.cc#newcode128 lily/relocate.cc:128: TOPLEVEL_VERSION, package_datadir)); Why no .c_str () here (and lots ...
2 months, 1 week ago (2019-02-08 09:42:27 UTC) #1
lemzwerg
On 2019/02/08 09:42:27, dak wrote: > https://codereview.appspot.com/347070043/diff/1/lily/relocate.cc > File lily/relocate.cc (right): > > https://codereview.appspot.com/347070043/diff/1/lily/relocate.cc#newcode128 > ...
2 months, 1 week ago (2019-02-08 09:49:58 UTC) #2
dak
On 2019/02/08 09:49:58, lemzwerg wrote: > On 2019/02/08 09:42:27, dak wrote: > > https://codereview.appspot.com/347070043/diff/1/lily/relocate.cc > ...
2 months, 1 week ago (2019-02-08 10:36:44 UTC) #3
lemzwerg
> Can you give any reason why it _should_ work rather than "I tried"? flower/include/international.hh:46
2 months, 1 week ago (2019-02-08 11:41:45 UTC) #4
dak
On 2019/02/08 11:41:45, lemzwerg wrote: > > Can you give any reason why it _should_ ...
2 months, 1 week ago (2019-02-08 12:05:10 UTC) #5
lemzwerg
> Frankly, my approach would be to throw that function out. It just > obfuscates ...
2 months, 1 week ago (2019-02-08 12:14:58 UTC) #6
dan_faithful.be
2 months, 1 week ago (2019-02-08 22:46:18 UTC) #7
On Feb 8, 2019, at 07:05, dak@gnu.org wrote:
> 
> string
> _f (char const *format, const string &s, const string &s2, const string
> &s3)
> {
>  return String_convert::form_string (gettext (format), s.c_str (),
> s2.c_str (),
>                                      s3.c_str ());
> }
> 
> anyway.  Frankly, my approach would be to throw that function out.  It
> just obfuscates what will and what will not work and keeps GCC from
> issuing warnings when number and uses of %s don't match.

+1 (but I’m not volunteering)
— 
Dan

Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b