Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Diff formatting shows weird whitespace characters on Windows 10 (� and ╖ instead of ·) #695

Open
jasenmoloy opened this issue Sep 13, 2020 · 6 comments
Labels

Comments

@jasenmoloy
Copy link

jasenmoloy commented Sep 13, 2020

Summary of problem

When outputting the details of what lines have format violations, the · characters representing whitespace are not rendering correctly in the Command Prompt, Android Studio & Intellij's console output on Windows 10. I tried the same project with the same IDEs and Gradle files on Mac OS Catalina and don't see the issue.

I use defaults fonts on Windows and in the IDEs. I tried other monospaced fonts and character encoding thinking it's something on my end. Didn't seem to have any effect. I'm suspecting something in the library is possibly using an encoding that isn't translating back to UTF-8 correctly.

Android Studio & Intellij's output (Windows 10)

Execution failed for task ':app:spotlessKotlinCheck'.
> The following files had format violations:
app\src\main\java\com\corwinjv\postmodernandroid\common\di\NetworkModule.kt
          @@ -6,15 +6,14 @@
           import�dagger.Provides
           import�dagger.hilt.InstallIn
           import�dagger.hilt.android.components.ApplicationComponent
          +import�javax.inject.Singleton
           import�okhttp3.OkHttpClient
           import�retrofit2.Retrofit
           import�retrofit2.converter.gson.GsonConverterFactory
          -import�javax.inject.Singleton
           @InstallIn(ApplicationComponent::class)
           @Module
          -class�NetworkModule
          -{
          +class�NetworkModule�{
           ����@Provides
           ����@Singleton
           ����fun�provideOkHttpClient():�OkHttpClient�{
          @@ -24,7 +23,7 @@
           ����@Provides
           ����@Singleton
          -����fun�provideProgrammingQuotesApi(okHttpClient:�OkHttpClient)�:�ProgrammingQuotesApi�{
          +����fun�provideProgrammingQuotesApi(okHttpClient:�OkHttpClient):�ProgrammingQuotesApi�{
           ��������return�Retrofit.Builder()
      ... (8 more lines that didn't fit)

Command Prompt's output (Windows 10)

Execution failed for task ':app:spotlessKotlinCheck'.
> The following files had format violations:
      app\src\main\java\com\corwinjv\postmodernandroid\common\di\NetworkModule.kt
          @@ -6,15 +6,14 @@
           import╖dagger.Provides
           import╖dagger.hilt.InstallIn
           import╖dagger.hilt.android.components.ApplicationComponent
          +import╖javax.inject.Singleton
           import╖okhttp3.OkHttpClient
           import╖retrofit2.Retrofit
           import╖retrofit2.converter.gson.GsonConverterFactory
          -import╖javax.inject.Singleton

           @InstallIn(ApplicationComponent::class)
           @Module
          -class╖NetworkModule
          -{
          +class╖NetworkModule╖{
           ╖╖╖╖@Provides
           ╖╖╖╖@Singleton
           ╖╖╖╖fun╖provideOkHttpClient():╖OkHttpClient╖{
          @@ -24,7 +23,7 @@

           ╖╖╖╖@Provides
           ╖╖╖╖@Singleton
          -╖╖╖╖fun╖provideProgrammingQuotesApi(okHttpClient:╖OkHttpClient)╖:╖ProgrammingQuotesApi╖{
          +╖╖╖╖fun╖provideProgrammingQuotesApi(okHttpClient:╖OkHttpClient):╖ProgrammingQuotesApi╖{
           ╖╖╖╖╖╖╖╖return╖Retrofit.Builder()
      ... (8 more lines that didn't fit)

Android Studio 4.0's output (Mac OS Catalina)

Execution failed for task ':app:spotlessKotlinCheck'.
> The following files had format violations:
      app/src/androidTest/java/com/corwinjv/postmodernandroid/ExampleInstrumentedTest.kt
          @@ -1,13 +1,11 @@
           package·com.corwinjv.postmodernandroid
           
          +import·androidx.test.ext.junit.runners.AndroidJUnit4
           import·androidx.test.platform.app.InstrumentationRegistry
          -import·androidx.test.ext.junit.runners.AndroidJUnit4
          -
          +import·org.junit.Assert.assertEquals
           import·org.junit.Test
           import·org.junit.runner.RunWith
           
          -import·org.junit.Assert.assertEquals
          -
           /**
           ·*·Instrumented·test,·which·will·execute·on·an·Android·device.
           ·*
      app/src/main/java/com/corwinjv/postmodernandroid/App.kt
          @@ -4,4 +4,4 @@
           import·dagger.hilt.android.HiltAndroidApp
           
           @HiltAndroidApp
          -class·App·:·Application()
          +class·App·:·Application()
      app/src/main/java/com/corwinjv/postmodernandroid/common/di/NetworkModule.kt
          @@ -6,15 +6,14 @@
           import·dagger.Provides
           import·dagger.hilt.InstallIn
           import·dagger.hilt.android.components.ApplicationComponent
          +import·javax.inject.Singleton
           import·okhttp3.OkHttpClient
           import·retrofit2.Retrofit
           import·retrofit2.converter.gson.GsonConverterFactory
          -import·javax.inject.Singleton
           
           @InstallIn(ApplicationComponent::class)
           @Module
          -class·NetworkModule
          -{
          +class·NetworkModule·{
           ····@Provides
           ····@Singleton
           ····fun·provideOkHttpClient():·OkHttpClient·{
          @@ -24,7 +23,7 @@
           
           ····@Provides
           ····@Singleton
          -····fun·provideProgrammingQuotesApi(okHttpClient:·OkHttpClient)·:·ProgrammingQuotesApi·{
          +····fun·provideProgrammingQuotesApi(okHttpClient:·OkHttpClient):·ProgrammingQuotesApi·{
           ········return·Retrofit.Builder()
      ... (8 more lines that didn't fit)

gradle or maven version

Gradle - 6.1.1

spotless version

5.5.1

operating system and version

  • Windows 10 Pro (19041.508)
  • Android Studio 4.0
  • Intellij IDEA 2019.3.4

copy-paste your full Spotless configuration block(s)

Root project Gradle file

buildscript {
    repositories {
        // ...
        maven { url "https://plugins.gradle.org/m2/" }
    }
    dependencies {
        // ...
        classpath 'com.android.tools.build:gradle:4.0.1'
        classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:1.4.0"
        classpath "com.diffplug.spotless:spotless-plugin-gradle:5.5.1"
    }
}

apply plugin: "com.diffplug.spotless"
// ...

Subproject (app)'s Gradle file

// ...
apply plugin: "com.diffplug.spotless"

spotless {
    kotlin {
        target 'src/*/java/**/*.kt'
        ktlint()
    }
}
// ...

copy-paste the full content of any console errors emitted by gradlew spotless[Apply/Check] --stacktrace

See summary

@nedtwigg nedtwigg added the bug label Sep 14, 2020
@nedtwigg
Copy link
Member

nedtwigg commented Sep 14, 2020

Thanks for testing in so many places. The formatted diff gets built here, and I'm pretty sure there are no encoding bugs here:

RawText a = new RawText(dirty.getBytes(StandardCharsets.UTF_8));
RawText b = new RawText(clean.getBytes(StandardCharsets.UTF_8));
EditList edits = new EditList();
edits.addAll(MyersDiff.INSTANCE.diff(RawTextComparator.DEFAULT, a, b));
ByteArrayOutputStream out = new ByteArrayOutputStream();
try (DiffFormatter formatter = new DiffFormatter(out)) {
formatter.format(edits, a, b);
}
String formatted = out.toString(StandardCharsets.UTF_8.name());

From the RawText javadoc:

Elements of the sequence are the lines of the file, as delimited by the UNIX newline character ('\n'). The file content is treated as 8 bit binary text, with no assumptions or requirements on character encoding.

After that, there's no more byte[] manipulation, just string concatenation which eventually gets passed as a string to the GradleException:

return new GradleException(DiffMessageFormatter.builder()
.runToFix("Run '" + calculateGradleCommand() + " " + getTaskPathPrefix() + "spotlessApply' to fix these violations.")
.formatter(formatter)
.problemFiles(problemFiles)
.getMessage());

So I'm pretty sure the encoding error is either Gradle, the JVM, or the IDE. Based on that, it seems like the only fix we can do from the spotless side is to make the MIDDLE_DOT platform/environment sensitive:

The problem as it stands is that Windows IntelliJ turns that into , and Windows CMD turns it into . When I google that CMD character, I get U+2556 (a box drawing character).

A few options:

  • safest: if FileSignature.machineIsWin(), then MIDDLE_DOT = ' '; (a normal, invisible space)
  • pretty safe: if FileSignature.machineIsWin(), then MIDDLE_DOT = '\u0095'; (U+2022 bullet, misencoded to 0x95 so that 1252 will recognize it, not sure if this will work)
    • \u0095 is technically a control character (message waiting), but sometimes it seems to rendered as a middle dot, so hopefully even if it is "correctly" interpreted as codepoint \u0095, even though we're really trying to trick it into be interpreted as codepoint \u2022 by old 1252 encodings, we should still be okay...
  • something like the above, but keyed off of some other JVM property

I'd be happy to merge a PR which implements the "safest" option above. If it's been tested in cmd and IntelliJ, then I'd be happy to merge the "less safe" option as well. And if anyone has expertise on a deeper fix, that sounds great too!

@nedtwigg nedtwigg changed the title Gradle spotlessKotlinCheck outputs invalid · characters on Windows 10 Diff formatting shows weird whitespace characters on Windows 10 (� and ╖ instead of ·) Sep 14, 2020
@jasenmoloy
Copy link
Author

jasenmoloy commented Sep 15, 2020

I'm happy to test and post updates to see what any option will result.

Minor Update

In adding a preBuild.dependsOn('spotlessCheck') line to ensure formatting is checked when compiling, I noticed different output behavior depending on the output windows I've selected in Android Studio. The Gradle task output appears to render correctly where the console appears broken. cmd shows no improvement.

Gradle Task Build Output
image

The following files had format violations:
    videoplayer\src\main\java\com\imgur\mobile\videoplayer\audio\AudioFocusManager.kt
        @@ -69,7 +69,8 @@
         private·class·InternalListener(private·val·listener:·AudioFocusManager.Listener)·:·AudioManager.OnAudioFocusChangeListener·{
         ····override·fun·onAudioFocusChange(focusChange:·Int)·{
         ········if·(focusChange·==·AudioManager.AUDIOFOCUS_LOSS·||
        -········focusChange·==·AudioManager.AUDIOFOCUS_LOSS_TRANSIENT)·{
        +············focusChange·==·AudioManager.AUDIOFOCUS_LOSS_TRANSIENT
        +········)·{
         ············listener.onAudioFocusLost()
         ········}
         ····}
Run 'gradlew.bat :videoplayer:spotlessApply' to fix these violations.

Overall Build Output
image

> The following files had format violations:
      videoplayer\src\main\java\com\imgur\mobile\videoplayer\audio\AudioFocusManager.kt
          @@ -69,7 +69,8 @@
           private�class�InternalListener(private�val�listener:�AudioFocusManager.Listener)�:�AudioManager.OnAudioFocusChangeListener�{
           ����override�fun�onAudioFocusChange(focusChange:�Int)�{
           ��������if�(focusChange�==�AudioManager.AUDIOFOCUS_LOSS�||
          -��������focusChange�==�AudioManager.AUDIOFOCUS_LOSS_TRANSIENT)�{
          +������������focusChange�==�AudioManager.AUDIOFOCUS_LOSS_TRANSIENT
          +��������)�{
           ������������listener.onAudioFocusLost()
           ��������}
           ����}
  Run 'gradlew.bat :videoplayer:spotlessApply' to fix these violations.

Definitely seems like it's something specific with Gradle or the JVM when outputting to the console.

@nedtwigg
Copy link
Member

nedtwigg commented Sep 16, 2020

Thanks for testing. The fact that gradle task output is correct, while the terminal build output is incorrect, is confirmation that the terminal encoding is the problem. I'm happy to intentionally misencode if we can reliably detect when to do it, but I don't see how we can. I did a super quick test, and with whatever default font I have on my mac, the \u0095 does the correct thing, and renders as completely invisible, not even taking up horizontal space, so it's even worse than �. Perhaps windows does better, you can integration test for yourself like this.

Based on this, I'm inclined to either:

  1. leave it as-is, and let the terminal people fix their terminal
  2. make this change: private static final char MIDDLE_DOT = FileSignature.machineIsWin() ? ' ' : '·';

I'd be happy to merge a PR from someone that implements 2, or maybe someone figures out a better way forward.

@JavierSegoviaCordoba
Copy link
Contributor

JavierSegoviaCordoba commented Oct 20, 2021

@nedtwigg I realized in gradle/gradle#18618 that symbols are written directly, they work, but if it using a code, they don't.

Have you tried private static final char MIDDLE_DOT = '·'; instead of private static final char MIDDLE_DOT = '\u00b7';?

@nedtwigg
Copy link
Member

I don't think that will help. If someone builds and tests something I'm happy to merge it, but I don't care about this issue.

@matspetter
Copy link

matspetter commented Oct 18, 2023

I can just confirm I have the same problem. Output on my Win10 machine:

           ╖╖╖╖}

           ╖╖╖╖override╖fun╖login(vin:╖String?):╖Single<Boolean>╖{
          -╖╖╖╖╖╖╖╖╖return╖Single.just(true)
          +╖╖╖╖╖╖╖╖return╖Single.just(true)
           ╖╖╖╖}

And something similar on our Ubuntu CICD machine:

          @@ -18,15 +18,12 @@
           ????}
           
           ????override?fun?login(vin:?String?):?Single<Boolean>?{
          -?????????return?Single.just(true)
          +????????return?Single.just(true)
           ????}

using:
spotless-plugin-gradle:6.22.0
gradle 7.5

Can it be connected to the encoding of the source code files?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants