Open Artawower opened 2 years ago
Hey, thanks for the pull request!
I researched a little, and it would be great if we could comply with Unist—all Node
have position
field that has start
and end
subfields. Doing so would allow us to use more of unified plugins (e.g., reporting messages/warnings on source file).
To convert offset to Point
, we can use #location
field on Reader
(a vfile-location
instance):
public toPoint(offset: number): Point {
return this.#location.toPoint(offset);
}
public toPosition(begin: number, end: number): Position {
return {
start: this.toPoint(begin),
end: this.toPoint(end),
};
}
That should also resolve end
collision on Keyword
.
Position information adds quite a lot of details and makes AST much bigger, so it would be nice if we would only add positional information if position
setting is set to true. (We need to add new setting.)
npm test -- --update
When I try to run the test from the root path, I get this error Cannot find module 'uniorg-parse/lib/parser'
from different packages. Do you know how to fix this?
I was running the test from the uniorg-parse package before, and it worked fine.
Try running npm run bootstrap && npm run build
first
Merging #50 (ba0581e) into master (c6fdf0d) will increase coverage by
0.01%
. The diff coverage is98.03%
.
@@ Coverage Diff @@
## master #50 +/- ##
==========================================
+ Coverage 95.93% 95.94% +0.01%
==========================================
Files 15 15
Lines 1622 1651 +29
Branches 522 532 +10
==========================================
+ Hits 1556 1584 +28
- Misses 65 66 +1
Partials 1 1
Impacted Files | Coverage Δ | |
---|---|---|
packages/uniorg-parse/src/parse-options.ts | 100.00% <ø> (ø) |
|
packages/uniorg-parse/src/parser.ts | 95.24% <97.87%> (+0.01%) |
:arrow_up: |
packages/uniorg-parse/src/reader.ts | 96.29% <100.00%> (+0.19%) |
:arrow_up: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more
Hey, sorry for the long review 🙃 the things have been a bit crazy over here
No problem, it really is a crazy time for all of us
Also i noticed that timestamp parsing was incorrect,
children:
- type: "clock"
value:
type: "timestamp"
timestampType: "inactive"
rawValue: "[2021-01-10 Sun 14:36]"
start:
year: 2021
month: 1
day: 10
hour: 14
minute: 36
end: null
position:
start:
line: 1
column: 8
offset: 7
end:
line: 1
column: 30
offset: 29
The real start should be 0. I added additional argument to parseTimestamp
function, that indicate about real offset before usage. I dunno might be there is better alternative to determine real offset
I had no luck setting begin/end for the Timestamp object (since this type already has an end property, I could use a universal property name - contentsBegin/contentsEnd, if you don't mind).