Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Interactive CLI and init command fixes #164

Merged
merged 2 commits into from
Mar 10, 2020
Merged

Interactive CLI and init command fixes #164

merged 2 commits into from
Mar 10, 2020

Conversation

sebmck
Copy link
Contributor

@sebmck sebmck commented Mar 10, 2020

cli-reporter: Add informative prompts for key usage on all select calls
cli-reporter: In select, when pressing escape, kill the process
cli-reporter: In select, when we kill the process, also output a warning message
local init command: Add support for tests component
local init command: Error when a project already exists in the cwd

Resolves #161 and #162

Screenshot from 2020-03-09 20-19-30
Screenshot from 2020-03-09 20-18-43
Screenshot from 2020-03-09 20-17-34

cli-reporter: In select, when pressing escape, kill the process
cli-reporter: In select, when we kill the process, also output a warning message
local init command: Add support for tests component
local init command: Error when a project already exists in the cwd
@Kelbie
Copy link
Contributor

Kelbie commented Mar 10, 2020

I'll close #161 because this fixes the issue but I have one small nit, could you put the warning on a new line?
Screenshot 2020-03-10 at 09 53 25

@MareikeTaeubner
Copy link

MareikeTaeubner commented Mar 10, 2020

I ran the init script in order to document the new behavior. My sample project already contained a rome.json file and I wanted to see how the script reacts to it. I got a result that does not look like your screenshot:

Unhandled CLI query error
DiagnosticsError: Expected to close filelink but found emphasis

 unknown:1:80 parse/stringMarkup ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ✖ Expected to close filelink but found emphasis

    \<filelink target="C:\Users\marei\src\rome-sample\rome.json" emphasis>rome.json\</emphasis> file already exists
                                                                                    ^^^^^^^^

- Expected to close filelink but found emphasis
  at ___R$project$rome$$romejs$diagnostics$errors_ts$createSingleDiagnosticError (C:\Users\marei\AppData\Local\Temp\rome-dev\index.js:52863:11) generated source location
  at StringMarkupParser.unexpected (C:\Users\marei\AppData\Local\Temp\rome-dev\index.js:1583:12) generated source location
  at StringMarkupParser.parseTag (C:\Users\marei\AppData\Local\Temp\rome-dev\index.js:2029:25) generated source location
  at StringMarkupParser.parseChild (C:\Users\marei\AppData\Local\Temp\rome-dev\index.js:2059:20) generated source location
  at StringMarkupParser.parse (C:\Users\marei\AppData\Local\Temp\rome-dev\index.js:2069:27) generated source location
  at ___R$project$rome$$romejs$string$markup$parse_ts$parseMarkup (C:\Users\marei\AppData\Local\Temp\rome-dev\index.js:2077:109) generated source location
  at ___R$$priv$project$rome$$romejs$string$markup$format_ts$formatReduceFromInput (C:\Users\marei\AppData\Local\Temp\rome-dev\index.js:3436:92) generated source location
  at ___R$project$rome$$romejs$string$markup$format_ts$markupToAnsi (C:\Users\marei\AppData\Local\Temp\rome-dev\index.js:3529:11) generated source location
  at ___R$project$rome$$romejs$cli$reporter$Reporter_ts$default.markupify (C:\Users\marei\AppData\Local\Temp\rome-dev\index.js:50452:15) generated source location
  at ___R$project$rome$$romejs$cli$reporter$Reporter_ts$default.logAllWithCategory (C:\Users\marei\AppData\Local\Temp\rome-dev\index.js:50496:31) generated source location
  at ___R$project$rome$$romejs$cli$reporter$Reporter_ts$default.error (C:\Users\marei\AppData\Local\Temp\rome-dev\index.js:50544:11) generated source location
  at Object.callback (C:\Users\marei\AppData\Local\Temp\rome-dev\index.js:53660:17) generated source location
  at ___R$project$rome$$romejs$core$client$ClientRequest_ts$default.initFromLocal (C:\Users\marei\AppData\Local\Temp\rome-dev\index.js:53892:22) generated source location
  at ___R$project$rome$$romejs$core$client$ClientRequest_ts$default.init (C:\Users\marei\AppData\Local\Temp\rome-dev\index.js:53861:15) generated source location
  at ___R$project$rome$$romejs$core$client$Client_ts$default.query (C:\Users\marei\AppData\Local\Temp\rome-dev\index.js:68885:18) generated source location
  at ___R$project$rome$$romejs$cli$cli_ts$default (C:\Users\marei\AppData\Local\Temp\rome-dev\index.js:71221:16) generated source location

This seems to be a bug, I could fix it for now with this, but the result does not look like yours (the link is not clickable):

 reporter.error(
        `<filelink emphasis target="${configPath.join()}" /> file already exists`,
      );

Copy link

@MareikeTaeubner MareikeTaeubner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The filelink tag is not properly closed on windows, the script then crashes when the rome.json already exists.

const configPath = req.client.flags.cwd.append('rome.json');
if (await exists(configPath)) {
reporter.error(
`<filelink target="${configPath.join()}" emphasis>rome.json</emphasis> file already exists`,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`<filelink target="${configPath.join()}" emphasis>rome.json</emphasis> file already exists`,
`<filelink emphasis target="${configPath.join()}" /> file already exists`,

@sebmck
Copy link
Contributor Author

sebmck commented Mar 10, 2020

Oops, I actually fixed that bug forgot to commit it.

@sebmck sebmck merged commit e36c03e into master Mar 10, 2020
@sebmck sebmck deleted the interactive-fixes branch March 10, 2020 20:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rome init creates rome.json file when prompt is escaped.
4 participants