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

Improve Mark display on compact mode #332

Closed
wants to merge 1 commit into from
Closed

Improve Mark display on compact mode #332

wants to merge 1 commit into from

Conversation

ooflorent
Copy link

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
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
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
Member

puzrin commented May 20, 2017

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

@rlidwka
Copy link
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