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

$tsc-watch background problem matcher is locale specific #46373

Closed
victorandree opened this issue Mar 22, 2018 · 11 comments
Closed

$tsc-watch background problem matcher is locale specific #46373

victorandree opened this issue Mar 22, 2018 · 11 comments
Assignees
Labels
feature-request Request for new features or functionality tasks Task system issues verified Verification succeeded
Milestone

Comments

@victorandree
Copy link

victorandree commented Mar 22, 2018

The beginsPattern and endsPattern for the $tsc-watch problem matcher match on lines where the time is reported in a specific format (11:12:13 PM). The tsc watch output, however, uses toLocaleTimeString which changes depending on your locale (e.g. 11:12:13 p.m.). This results in the task never ending if your locale is wrong.

From extensions/typescript/package.json:

{
  "background": {
    "activeOnStart": true,
    "beginsPattern": {
      "regexp":
        "^\\s*(?:message TS6032:|\\d{1,2}:\\d{1,2}:\\d{1,2}(?: AM| PM)? -) File change detected\\. Starting incremental compilation\\.\\.\\."
    },
    "endsPattern": {
      "regexp":
        "^\\s*(?:message TS6042:|\\d{1,2}:\\d{1,2}:\\d{1,2}(?: AM| PM)? -) Compilation complete\\. Watching for file changes\\."
    }
  }
}

Output from tsc-watch on my machine is:

11:41:10 p.m. - Compilation complete. Watching for file changes.

A quick test shows that this doesn't match the endsPattern in package.json, because my AM/PM is capitalized the way it is. If I create a custom matcher, the task finishes successfully.

Is it necessary to match the full line? If not, simply matching the end of each line might be sufficient (expressions not JSON escaped).

beginsPattern: File change detected\. Starting incremental compilation\.\.\.$
endsPattern: Compilation complete\. Watching for file changes\.$

It's probably not a fun project to try and build a regex capturing all different ways to format times, so maybe the liberal ^\s*(?:message TS6042:|.*? -) could work if you want a pattern for the full line?

I'm running the latest version of relevant software:

Component Version
Visual Studio Code 1.21.1
TypeScript 2.7.2
 macOS High Sierra 10.13.3
Locale English with Swedish formats*

* This format seems pretty Node/v8 specific, because I really use 24 hour time and in Language & Region in System Preferences, I do have AM/PM as the chosen before/after noon markers. If run new Date().toLocaleTimeString() in node, I get the above formatting (node 9.8.0) -- but in Chrome it's 24 hour time.

Here's a full tasks.json which could work for other people:

{
  "version": "2.0.0",
  "tasks": [
    {
      "type": "typescript",
      "tsconfig": "tsconfig.json",
      "option": "watch",
      "isBackground": true,
      "presentation": {
        "echo": true,
        "reveal": "always",
        "focus": false,
        "panel": "shared"
      },
      "problemMatcher": {
        "base": "$tsc-watch",
        "background": {
          "activeOnStart": true,
          "beginsPattern":
            "File change detected\\. Starting incremental compilation\\.\\.\\.$",
          "endsPattern": "Compilation complete\\. Watching for file changes\\.$"
        }
      },
      "group": {
        "kind": "build",
        "isDefault": true
      }
    }
  ]
}

I'm pretty sure the diagnostic is generated by TypeScript in createWatchStatusReporter in src/compiler/watch.ts:

export function createWatchStatusReporter(system: System, pretty?: boolean): WatchStatusReporter {
    return pretty ?
        (diagnostic, newLine, options) => {
            clearScreenIfNotWatchingForFileChanges(system, diagnostic, options);
            let output = `[${formatColorAndReset(new Date().toLocaleTimeString(), ForegroundColorEscapeSequences.Grey)}] `;
            output += `${flattenDiagnosticMessageText(diagnostic.messageText, system.newLine)}${newLine + newLine + newLine}`;
            system.write(output);
        } :
        (diagnostic, newLine, options) => {
            clearScreenIfNotWatchingForFileChanges(system, diagnostic, options);
            let output = new Date().toLocaleTimeString() + " - ";
            output += `${flattenDiagnosticMessageText(diagnostic.messageText, system.newLine)}${newLine + newLine + newLine}`;
            system.write(output);
        };
}
@vscodebot
Copy link

vscodebot bot commented Mar 22, 2018

(Experimental duplicate detection)
Thanks for submitting this issue. Please also check if it is already covered by an existing one, like:

@vscodebot vscodebot bot added the tasks Task system issues label Mar 22, 2018
@dbaeumer
Copy link
Member

For now I will add a.m. and p.m. to the patterns. If we detect more locale variants for am / pm we need to investigate if we go with a different solution.

@victorandree
Copy link
Author

victorandree commented Mar 26, 2018

I did a quick check with the locales found in this StackOverflow post check using new Date().toLocaleTimeString(locale) on node/Chrome. Even for English, you also have am (en-AU), but then you have Arabic, Greek, Spanish (as spoken in US), Mandarin and others... Here's the different formats I saw:

  • 12:06:02
  • 12:06:02 ከሰዓት
  • ١٢:٠٦:٠٢ م
  • 12:06:02 م
  • 12:06:02 ч.
  • ১২:০৬:০২ PM
  • 12.06.02
  • 12:06:02 μ.μ.
  • 12:06:02 PM
  • 12:06:02 pm
  • 12:06:02 p.m.
  • 12:06:02 p. m.
  • ۱۲:۰۶:۰۲
  • 12 h 06 min 02 s
  • 12:06:02 ಅಪರಾಹ್ನ
  • 오후 12:06:02
  • १२:०६:०२ म.उ.
  • 12:06:02 PTG
  • பிற்பகல் 12:06:02
  • 下午12:06:02

@dbaeumer
Copy link
Member

@victorandree thanks. I will reopen the issue and see what a better solution would be.

@dbaeumer dbaeumer reopened this Mar 27, 2018
@dbaeumer dbaeumer added this to the On Deck milestone Mar 27, 2018
@dbaeumer dbaeumer added the bug Issue identified by VS Code Team member as probable bug label Mar 27, 2018
@alexr00 alexr00 modified the milestones: On Deck, Backlog Jan 4, 2019
@alexr00 alexr00 added feature-request Request for new features or functionality and removed bug Issue identified by VS Code Team member as probable bug labels Jan 4, 2019
@alexr00
Copy link
Member

alexr00 commented Jan 4, 2019

Supporting localized problem matchers is more of a feature than a bug.

@alexr00
Copy link
Member

alexr00 commented Nov 19, 2019

@mjbvz I've gotten a whole bunch of these issues recently. Do you think it would be safe to match on just the the [hh:mm:ss] pattern and accept any other characters before or after the times? That would capture most of these times.

@mjbvz
Copy link
Contributor

mjbvz commented Nov 19, 2019

That sounds reasonable to me. Do you want to make the change or should I?

@alexr00
Copy link
Member

alexr00 commented Nov 20, 2019

Sweet. I made the change and confirmed that English still works. I'll follow up with the people who hit this in other issues after we have an Insiders build and see if they can verify it.

@alexr00
Copy link
Member

alexr00 commented Feb 4, 2020

The tsc problem matchers can't be made completely locale agnostic, but closing since this should be improved over the previous behavior.

@Tyriar
Copy link
Member

Tyriar commented Feb 25, 2020

@alexr00 please add a test plan or verification needed

@alexr00 alexr00 added the verification-needed Verification of issue is requested label Feb 25, 2020
@alexr00
Copy link
Member

alexr00 commented Feb 25, 2020

To verify, make sure that problems in English can still be detected with the $tsc-watch problem matcher. If you happen to also have a machine with a language other than English that uses one of the other separators, verify there too (not required).

@connor4312 connor4312 added verified Verification succeeded and removed verification-needed Verification of issue is requested labels Feb 26, 2020
@vscodebot vscodebot bot locked and limited conversation to collaborators Mar 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality tasks Task system issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

6 participants