Re: ant git commit: Yet more isEmpty()

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: ant git commit: Yet more isEmpty()

Stefan Bodewig
On 2018-04-15, <[hidden email]> wrote:

> @@ -275,10 +270,7 @@ public class DefaultLogger implements BuildLogger {
>                      new BufferedReader(new StringReader(event.getMessage()))) {

>                      message.append(r.lines().map(line -> prefix + line)
> -                        .collect(Collectors.joining(StringUtils.LINE_SEP)));
> -                    if (message.length() == 0) {
> -                        message.append(prefix);
> -                    }
> +                        .collect(Collectors.joining(System.lineSeparator(), prefix, "")));
>                  } catch (IOException e) {
>                      // shouldn't be possible
>                      message.append(label).append(event.getMessage());

I think this puts the last "prefix" in the wrong position, In the
original code it was appended after the message, the new joining call
puts it before the joined message. The second and third arg of joining
need to be swapped IMHO.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: ant git commit: Yet more isEmpty()

Gintautas Grigelionis
2018-04-15 10:04 GMT+00:00 Stefan Bodewig <[hidden email]>:

> On 2018-04-15, <[hidden email]> wrote:
>
> > @@ -275,10 +270,7 @@ public class DefaultLogger implements BuildLogger {
> >                      new BufferedReader(new
> StringReader(event.getMessage()))) {
>
> >                      message.append(r.lines().map(line -> prefix + line)
> > -                        .collect(Collectors.joining(
> StringUtils.LINE_SEP)));
> > -                    if (message.length() == 0) {
> > -                        message.append(prefix);
> > -                    }
> > +                        .collect(Collectors.joining(System.lineSeparator(),
> prefix, "")));
> >                  } catch (IOException e) {
> >                      // shouldn't be possible
> >                      message.append(label).append(event.getMessage());
>
> I think this puts the last "prefix" in the wrong position, In the
> original code it was appended after the message, the new joining call
> puts it before the joined message. The second and third arg of joining
> need to be swapped IMHO.
>

Good catch! I believe I have a better idea, though :->

https://github.com/apache/ant/commit/cc8563cdf535edd0a94d5dbfe2113ffebbb961df

Gintas