-
Notifications
You must be signed in to change notification settings - Fork 11
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
Add file line numbers to errors #48
Conversation
Signed-off-by: Saswata Mukherjee <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! One suggestion
@@ -48,9 +48,9 @@ func NewChain(c ...mdformatter.LinkTransformer) mdformatter.LinkTransformer { | |||
return &chain{chain: c} | |||
} | |||
|
|||
func (l *chain) TransformDestination(ctx mdformatter.SourceContext, destination []byte) (_ []byte, err error) { | |||
func (l *chain) TransformDestination(ctx mdformatter.SourceContext, destination []byte, lines string) (_ []byte, err error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lines of what? do you mean lineNumber?
I find it very confusing we need to pass lines through transformers args 🤔
Why not adding it to SourceContext
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't think of that. Will try to implement! 🙂
Signed-off-by: Saswata Mukherjee <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Almost there ;p
@@ -75,6 +78,7 @@ func (t *transformer) Render(w io.Writer, source []byte, node ast.Node) error { | |||
if token.Attr[i].Key != "src" { | |||
continue | |||
} | |||
t.sourceCtx.LineNumbers = getLinkLines(source, []byte(token.Attr[i].Val), t.frontMatterLen) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am pretty sure we can get this directly from node (...) offset
🤔
Traversing huge file might get us quite latency problems... can we try that? 🤗
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure! Will try to implement this! 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't seem to find any offset
method/field which could give line numbers for a particular node. I could only find this method in text
package but functionality seems to be different...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It actually shows the offset in source 🤔 usually ;p
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not fan of this reparsing source....
But should work for now. Let's optimize if needed later 👍🏽
This PR adds line numbers to errors in the following way,
Fixes #36.