Skip to content

Improve Mark display on compact mode#332

Closed
ooflorent wants to merge 1 commit intonodeca:masterfrom
ooflorent:compact_mark
Closed

Improve Mark display on compact mode#332
ooflorent wants to merge 1 commit intonodeca:masterfrom
ooflorent:compact_mark

Conversation

@ooflorent
Copy link
Copy Markdown

Before:

in "/Users/fc/Development/js-yaml/test/units/mark.txt" at line 1, column 1

After:

in "/Users/fc/js-yaml/test/units/mark.txt" (1:1)

@puzrin
Copy link
Copy Markdown
Member

puzrin commented Mar 2, 2017

  1. This can be a breaking change, if someone analyze message.
  2. I don't like format (x:y) - message become unclear for users.

Could you explain reasons for this PR?

@ooflorent
Copy link
Copy Markdown
Author

ooflorent commented Mar 6, 2017

  1. Sure. But is it a big deal? We could say the same thing if a sentence would have been reworded.
  2. This is the format adopted by babel

The reason behind this change is cosmetic. I have built a tool to format YAML files and errors are handled this way:

try {
  data = yaml.load(contents)
} catch (e) {
  const line = e.mark.line + 1
  const column = e.mark.column + 1
  console.error(`${e.name}: ${e.reason} (${line}:${column})\n`)
  console.error(codeFrame(contents, line, column))
  process.exit(1)
}

yml_fmt

The goal is to be able to switch to compact mode to prevent the code frame from being displayed but keeping the mark concise:

  • Using YAMLException#toString() dumps code 👎
  • Using YAMLException#toString(true) hides location 👎

Modifying Mark#toString(true) to be less verbose and adding it to YAMLException's compact output could be a good tradeoff between debugging info and length.

@puzrin
Copy link
Copy Markdown
Member

puzrin commented May 20, 2017

Let's keep this open, to revisit on major version bump.

@rlidwka
Copy link
Copy Markdown
Member

rlidwka commented Dec 17, 2020

Fixed in a58d8bc (together with stylus-like exception formatting).

@rlidwka rlidwka closed this Dec 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants