Don't count terminating \0 in Source_file::length

Don't count terminating \0 in Source_file::length

This can confuse the SCM parser, because GUILE can interpret a \0 as
part of an identifer.

[Dan Eble, 2020-02-14 12:43:40]

https://codereview.appspot.com/579310043/diff/549550043/lily/include/source-f...
File lily/include/source-file.hh (right):

https://codereview.appspot.com/579310043/diff/549550043/lily/include/source-f...
lily/include/source-file.hh:44: /* The input data, plus an extra \0 to terminate
*/
I haven't looked through this code.  Would changing this from std::vector<char>
to std::string be appropriate?

[dak, 2020-02-14 13:00:33]

https://codereview.appspot.com/579310043/diff/549550043/lily/source-file.cc
File lily/source-file.cc (right):

https://codereview.appspot.com/579310043/diff/549550043/lily/source-file.cc#n...
lily/source-file.cc:51: characters_.push_back ((char)c);
Frankly, this seems like C++ should offer something better for reading a whole
input file into a buffer (also it seems like the sort of thing that would be
needed on any non-random access file, not just standard input).

[hanwenn, 2020-02-14 22:12:28]

https://codereview.appspot.com/579310043/diff/549550043/lily/include/source-f...
File lily/include/source-file.hh (right):

https://codereview.appspot.com/579310043/diff/549550043/lily/include/source-f...
lily/include/source-file.hh:44: /* The input data, plus an extra \0 to terminate
*/
On 2020/02/14 12:43:40, Dan Eble wrote:
> I haven't looked through this code.  Would changing this from
std::vector<char>
> to std::string be appropriate?

Done.

https://codereview.appspot.com/579310043/diff/549550043/lily/source-file.cc
File lily/source-file.cc (right):

https://codereview.appspot.com/579310043/diff/549550043/lily/source-file.cc#n...
lily/source-file.cc:51: characters_.push_back ((char)c);
On 2020/02/14 13:00:33, dak wrote:
> Frankly, this seems like C++ should offer something better for reading a whole
> input file into a buffer (also it seems like the sort of thing that would be
> needed on any non-random access file, not just standard input).

Acknowledged.

[hanwenn, 2020-02-14 22:13:46]

std:string

[dak, 2020-02-14 22:40:52]

https://codereview.appspot.com/579310043/diff/561450043/lily/source-file.cc
File lily/source-file.cc (right):

https://codereview.appspot.com/579310043/diff/561450043/lily/source-file.cc#n...
lily/source-file.cc:137: data_ = string (&chars[0], chars.size ());
gulp_file_to_string ?

That one apparently also does some kind of logging, so one should check that
using it does not lead to duplications in the log.

[hanwenn, 2020-02-14 22:43:36]

https://codereview.appspot.com/579310043/diff/561450043/lily/source-file.cc
File lily/source-file.cc (right):

https://codereview.appspot.com/579310043/diff/561450043/lily/source-file.cc#n...
lily/source-file.cc:137: data_ = string (&chars[0], chars.size ());
On 2020/02/14 22:40:51, dak wrote:
> gulp_file_to_string ?
> 
> That one apparently also does some kind of logging, so one should check that
> using it does not lead to duplications in the log.

I prefer keeping the functionality unchanged. We'd print filenames twice if we
used gulp_file_to_string, because Includable_lexer::new_input also prints
[FILENAME .. ]

[hanwenn, 2020-02-14 23:13:48]

remove vector<char>

[Dan Eble, 2020-02-15 13:53:17]

LGTM.  Changing to std::string is a definite improvement.  Thanks.

https://codereview.appspot.com/579310043/diff/557420043/lily/engraver.cc
File lily/engraver.cc (right):

https://codereview.appspot.com/579310043/diff/557420043/lily/engraver.cc#newc...
lily/engraver.cc:121: if (!scm_is_pair (props)) {
misplaced braces

https://codereview.appspot.com/579310043/diff/557420043/lily/engraver.cc#newc...
lily/engraver.cc:124: };
superfluous semicolon

https://codereview.appspot.com/579310043/diff/557420043/lily/source-file.cc
File lily/source-file.cc (right):

https://codereview.appspot.com/579310043/diff/557420043/lily/source-file.cc#n...
lily/source-file.cc:55: return contents of FILENAME. *Not 0-terminated!*
update

https://codereview.appspot.com/579310043/diff/557420043/lily/source-file.cc#n...
lily/source-file.cc:84: string dest;
I'd construct with (count, character) rather than construct empty and resize.

[hanwenn, 2020-02-15 17:06:34]

rebase

[hanwenn, 2020-02-15 17:07:00]

https://codereview.appspot.com/579310043/diff/557420043/lily/engraver.cc
File lily/engraver.cc (right):

https://codereview.appspot.com/579310043/diff/557420043/lily/engraver.cc#newc...
lily/engraver.cc:121: if (!scm_is_pair (props)) {
On 2020/02/15 13:53:16, Dan Eble wrote:
> misplaced braces

Acknowledged.

https://codereview.appspot.com/579310043/diff/557420043/lily/engraver.cc#newc...
lily/engraver.cc:124: };
On 2020/02/15 13:53:16, Dan Eble wrote:
> superfluous semicolon

rebase artefact.

[hanwenn, 2020-02-15 17:07:22]

https://codereview.appspot.com/579310043/diff/557420043/lily/source-file.cc
File lily/source-file.cc (right):

https://codereview.appspot.com/579310043/diff/557420043/lily/source-file.cc#n...
lily/source-file.cc:55: return contents of FILENAME. *Not 0-terminated!*
On 2020/02/15 13:53:17, Dan Eble wrote:
> update

Done.

https://codereview.appspot.com/579310043/diff/557420043/lily/source-file.cc#n...
lily/source-file.cc:84: string dest;
On 2020/02/15 13:53:17, Dan Eble wrote:
> I'd construct with (count, character) rather than construct empty and resize.

Done.

[hanwenn, 2020-02-19 09:36:04]

commit 73a39333690293208b969d1d8d656f5837467e1c
Author: Han-Wen Nienhuys <hanwen@lilypond.org>
Date:   Thu Feb 13 09:13:02 2020 +0100

    Store Source_file data in std::string