REPL tool implementation code review

[Paul Sandoz]

On 7 Sep 2015, at 11:43, Remi Forax <forax at univ-mlv.fr> wrote:

> one comment on Paul's comments :)
> 

Last one, promise :-)

> ----- Mail original -----
>> De: "Paul Sandoz" <paul.sandoz at oracle.com>
>> Cc: kulla-dev at openjdk.java.net
>> Envoyé: Lundi 7 Septembre 2015 10:14:43
>> Objet: Re: REPL tool implementation code review
>> 
>> Hi,
>> 
>> Some comments on Remi’s comments, don’t worry the set keeps getting smaller
>> :-)
>> 
>> 
>> On 4 Sep 2015, at 20:46, Remi Forax <forax at univ-mlv.fr> wrote:
>>>> 
>>>> jdk/internal/jshell/tool/ExternalEditor.java:134
>>>>> private void saveFile() {
>>>>>   List<String> lines;
>>>>>   try {
>>>>>       lines = Files.readAllLines(tmpfile);
>>>>>   } catch (IOException ex) {
>>>>>       errorHandler.accept("Failure read edit file: " + ex.getMessage());
>>>>>       return ;
>>>>>   }
>>>>>   StringBuilder sb = new StringBuilder();
>>>>>   for (String ln : lines) {
>>>>>       sb.append(ln);
>>>>>       sb.append('\n');
>>>>>   }
>>>>>   saveHandler.accept(sb.toString());
>>>>> }
>>>> 
>>>> Why don’t you just read as bytes then create a String from those bytes?
>>> 
>>> or Files.lines(tmpFile).collect(Collectors.joining("\n”))
>>> 
>> 
>> I thought that initially too, but then IIRC the list of lines is consumed to
>> reproduce the file as a string, AFAICT the only difference being is a CR/LF
>> is guaranteed at the end.
> 
> with '\n' at the end of the last line:
>  Files.lines(tmpFile).collect(Collectors.joining("\n", "", "\n”)

Yes, you can do that, but my point is there may be no need to create a list of lines only to then reconstitute as a string containing those lines using a string builder, but perhaps there is some normalisation of line feeds going on?

Paul.