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

Spans for nested map values are needlessly multi-line #51

Open
kevmoo opened this issue Apr 9, 2019 · 4 comments
Open

Spans for nested map values are needlessly multi-line #51

kevmoo opened this issue Apr 9, 2019 · 4 comments
Labels
type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@kevmoo
Copy link
Member

kevmoo commented Apr 9, 2019

Accessing the span for a node in a nested map renders it (needlessly) multiline if it's the LAST item in a map.

Not an issue for root items, though...

import 'dart:convert';

import 'package:yaml/yaml.dart';

void main() {
  final yaml = loadYaml(const JsonEncoder.withIndent(' ').convert({
    'num': 42,
    'nested': {
      'null': null,
      'num': 42,
    },
    'null': null,
  })) as YamlMap;

  print(yaml.nodes['num'].span.message('hello'));

  final nestedNode = yaml.nodes['nested'] as YamlMap;

  print(nestedNode.nodes['null'].span.message('prints fine. not last'));
  print(nestedNode.nodes['num'].span.message('last value prints multi-line'));

  print(yaml.nodes['null'].span.message('top-level last value prints fine'));
}

output

line 2, column 9: hello
  ╷
2 │  "num": 42,
  │         ^^
  ╵
line 4, column 11: prints fine. not last
  ╷
4 │   "null": null,
  │           ^^^^
  ╵
line 5, column 10: last value prints multi-line
  ╷
5 │     "num": 42
  │ ┌──────────^
6 │ │  },
  │ └─^
  ╵
line 7, column 10: top-level last value prints fine
  ╷
7 │  "null": null
  │          ^^^^
  ╵
@kevmoo kevmoo added the type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) label Apr 9, 2019
kevmoo added a commit that referenced this issue Apr 9, 2019
kevmoo added a commit that referenced this issue Apr 9, 2019
Fixes #51

Also remove dead line from changelog
kevmoo added a commit that referenced this issue Apr 9, 2019
kevmoo added a commit that referenced this issue Apr 9, 2019
Fixes #51

Also remove dead line from changelog
@kevmoo kevmoo closed this as completed in #52 Apr 9, 2019
kevmoo added a commit that referenced this issue Apr 9, 2019
kevmoo added a commit that referenced this issue Apr 9, 2019
Fixes #51

Also remove dead line from changelog
kevmoo added a commit that referenced this issue Apr 10, 2019
This reverts commit aa6b82c.

#51 only seems to affect
JSON-encoded source files. Not worth this hack.
@kevmoo
Copy link
Member Author

kevmoo commented Apr 10, 2019

Fix was reverted in a0ecc98

@kevmoo kevmoo reopened this Apr 10, 2019
@adarshm-26
Copy link
Contributor

Hi @kevmoo, the bug seems to be due to scanner returning a token with incorrect span. The _scanPlainScalar moves ahead after finding a blank or break and modifies the 'end' variable responsible for identifying total span of the token. I have tried to break out of the _scanPlainScalar while loop if no plain character is found (adarshm-26@e816122) and it does fix this issue.
Also a beginner contributing to open source, so are there any particular steps I should follow before posting a PR in this repository ?? I messed up the last time 😛 .

@kevmoo
Copy link
Member Author

kevmoo commented Mar 2, 2020

Include tests, make sure travis is green, that's a good start

@adarshm-26
Copy link
Contributor

adarshm-26 commented Apr 20, 2020

The bug is more pronounced when using a raw yaml string,
Code:

import 'package:yaml/yaml.dart';
void main() {
  const dtr = 
  '''
  'nested_0': {
    'nested_1': {
      'nested_2': {
        'n2k1': null
        ,
        'n2k2': a
        ,
        'n2k3':                        4     
      },
      'n1k2': 425
    },
    'n0k2': aval      

                      ,
  }
  'rk2': 44
        ,
  ''';

  final yaml = loadYaml(dtr) as YamlMap;
  final yaml_n1 = yaml.nodes['nested_0'] as YamlMap;
  final yaml_n2 = yaml_n1.nodes['nested_1'] as YamlMap;
  final yaml_n3 = yaml_n2.nodes['nested_2'] as YamlMap;
  print(yaml.nodes['rk2'].span.message('Root last entry span, trailing space upto next line'));
  print(yaml_n1.nodes['n0k2'].span.message('Level 1 nested last entry span, trailing space upto comma(,)'));
  print(yaml_n2.nodes['n1k2'].span.message('Level 2 nested last entry span, trailing space upto curly brace(})'));
  print(yaml_n3.nodes['n2k1'].span.message('Level 3 nested first entry span, trailing space upto comma(,)'));
  print(yaml_n3.nodes['n2k2'].span.message('Level 3 nested mid entry span, trailing space upto comma(,)'));
  print(yaml_n3.nodes['n2k3'].span.message('Level 3 nested last entry span, trailing space upto curly brace(})'));
}

Output

line 16, column 10: Root last entry span, trailing space upto next line
   ╷
16 │     'rk2': 44
   │ ┌──────────^
17 │ └         ,
   ╵
line 12, column 13: Level 1 nested last entry span, trailing space upto comma(,)
   ╷
12 │       'n0k2': aval      
   │ ┌─────────────^
13 │ │ 
14 │ │                       ,
   │ └──────────────────────^
   ╵
line 10, column 15: Level 2 nested last entry span, trailing space upto curly brace(})
   ╷
10 │         'n1k2': 425
   │ ┌───────────────^
11 │ │     },
   │ └────^
   ╵
line 4, column 17: Level 3 nested first entry span, trailing space upto comma(,)
  ╷
4 │           'n2k1': null
  │ ┌─────────────────^
5 │ │         ,
  │ └────────^
  ╵
line 6, column 17: Level 3 nested mid entry span, trailing space upto comma(,)
  ╷
6 │           'n2k2': a
  │ ┌─────────────────^
7 │ │         ,
  │ └────────^
  ╵
line 8, column 40: Level 3 nested last entry span, trailing space upto curly brace(})
  ╷
8 │           'n2k3':                        4     
  │ ┌────────────────────────────────────────^
9 │ │       },
  │ └──────^
  ╵

the spec says - "Flow collections entries are terminated by the "," indicator. The final "," may be omitted". This might imply that their span should extend to the "," indicator, but in its absence it extends upto curly braces. The fix I suggested is wrong in this case, removing the PR. Treating the last entry separately could help.

mosuem pushed a commit to dart-lang/tools that referenced this issue Dec 11, 2024
mosuem pushed a commit to dart-lang/tools that referenced this issue Dec 11, 2024
mosuem pushed a commit to dart-lang/tools that referenced this issue Dec 11, 2024
This reverts commit cb3cf76.

dart-lang/yaml#51 only seems to affect
JSON-encoded source files. Not worth this hack.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

2 participants