Fixing most of the comment errors. (#152)

Author: jf-tech

customfuncs/customFuncs.go

@@ -8,7 +8,7 @@ import (
 	"github.com/jf-tech/omniparser/transformctx"
 )
 
-// CustomFuncType is the type of a custom function. Has to use interface{} given we support
+// CustomFuncType is the type of custom function. Has to use interface{} given we support
 // non-variadic and variadic functions.
 type CustomFuncType = interface{}
 
@@ -43,7 +43,7 @@ var CommonCustomFuncs = map[string]CustomFuncType{
 }
 
 // Coalesce returns the first non-empty string of the input strings. If no input strings are given or
-// all of them are empty, then empty string is returned. Note: a blank string (with only whitespaces)
+// all of them are empty, then an empty string is returned. Note: a blank string (with only whitespaces)
 // is not considered as empty.
 func Coalesce(_ *transformctx.Ctx, strs ...string) (string, error) {
 	for _, str := range strs {
@@ -54,7 +54,7 @@ func Coalesce(_ *transformctx.Ctx, strs ...string) (string, error) {
 	return "", nil
 }
 
-// Concat custom_func concatenates a number of strings together. If no strings specified, "" is returned.
+// Concat concatenates a number of strings together. If no strings specified, "" is returned.
 func Concat(_ *transformctx.Ctx, strs ...string) (string, error) {
 	var w strings.Builder
 	for _, s := range strs {

customfuncs/datetime.go

@@ -23,7 +23,7 @@ const (
 // If layout is specific, then parseDateTime will parse datetime string using the supplied layout;
 // otherwise, it will default to times.SmartParse.
 //
-// If datetime string contains tz info in it (such as 'Z', or '-America/New_York' etc, or '-0700', etc)
+// If datetime string contains tz info in it (such as 'Z', or '-America/New_York' etc., or '-0700', etc.)
 // then fromTZ is IGNORED. Otherwise, then datetime string will be parsed in with its face value y/m/d/h/m/s
 // and bonded with the fromTZ, if fromTZ isn't "".
 //

extensions/omniv21/customfuncs/javascript.go

@@ -15,7 +15,7 @@ const (
 	argNameNode = "_node"
 )
 
-// JSProgramCache caches *goja.Program. A *goja.Program is compiled javascript and it can be used
+// JSProgramCache caches *goja.Program. A *goja.Program is compiled javascript, and it can be used
 // across multiple goroutines and across different *goja.Runtime. If default loading cache capacity
 // is not desirable, change JSProgramCache to a loading cache with a different capacity at package
 // init time. Be mindful this will be shared across all use cases inside your process.
@@ -29,7 +29,7 @@ var jsRuntimePool sync.Pool
 // NodeToJSONCache caches *idr.Node to JSON translations.
 var NodeToJSONCache *caches.LoadingCache
 
-// For debugging/testing purpose so we can easily disable all the caches. But not exported. We always
+// For debugging/testing purpose, so we can easily disable all the caches. But not exported. We always
 // want caching in production.
 var disableCaching = false
 

extensions/omniv21/fileformat/csv/reader.go

@@ -20,7 +20,7 @@ type ErrInvalidHeader string
 
 func (e ErrInvalidHeader) Error() string { return string(e) }
 
-// IsErrInvalidHeader checks if an err is of ErrInvalidHeader type.
+// IsErrInvalidHeader checks if the `err` is of ErrInvalidHeader type.
 func IsErrInvalidHeader(err error) bool {
 	switch err.(type) {
 	case ErrInvalidHeader:
@@ -114,7 +114,7 @@ func (r *reader) recordToNode(record []string) *idr.Node {
 	root := idr.CreateNode(idr.DocumentNode, "")
 	// - If actual record has more columns than declared in schema, we'll only use up to
 	//   what's declared in the schema;
-	// - conversely, if the actual record has less columns than declared in schema, we'll
+	// - conversely, if the actual record has fewer columns than declared in schema, we'll
 	//   use all that are in the record.
 	for i := 0; i < maths.MinInt(len(record), len(r.decl.Columns)); i++ {
 		col := idr.CreateNode(idr.ElementNode, r.decl.Columns[i].name())

extensions/omniv21/fileformat/edi/reader.go

@@ -231,12 +231,6 @@ func (r *ediReader) Read() (*idr.Node, error) {
 		if err != nil {
 			return nil, err
 		}
-		if len(r.stack) <= 1 {
-			// we have a raw segment waiting for processing but currently
-			// the decl stack is effectively empty (with only the artificial
-			// #root decl on it. We can get into this situation two ways:
-			// 1.
-		}
 		cur := r.stackTop()
 		if !cur.segDecl.matchSegName(rawSeg.Name) {
 			if len(r.stack) <= 1 {

extensions/omniv21/fileformat/edi/reader2.go

@@ -16,7 +16,7 @@ type ErrInvalidEDI string
 
 func (e ErrInvalidEDI) Error() string { return string(e) }
 
-// IsErrInvalidEDI checks if an err is of ErrInvalidEDI type.
+// IsErrInvalidEDI checks if the `err` is of ErrInvalidEDI type.
 func IsErrInvalidEDI(err error) bool {
 	switch err.(type) {
 	case ErrInvalidEDI:
@@ -139,9 +139,9 @@ func (r *NonValidatingReader) Read() (RawSeg, error) {
 	r.segCount++
 	// We are here because:
 	// 1. we find next token (i.e. segment), great, let's process it, OR
-	// 2. r.scanner.Scan() returns false and it's EOF (note scanner never returns EOF, it just returns false
+	// 2. r.scanner.Scan() returns false, and it's EOF (note scanner never returns EOF, it just returns false
 	//    on Scan() and Err() returns nil). We need to return EOF, OR
-	// 3. r.scanner.Scan() returns false Err() returns err, need to return the err wrapped.
+	// 3. r.scanner.Scan() returns false Err() returns err, need to return the `err` wrapped.
 	err := r.scanner.Err()
 	if err != nil {
 		return RawSeg{}, ErrInvalidEDI(fmt.Sprintf("cannot read segment, err: %s", err.Error()))
@@ -167,7 +167,7 @@ func (r *NonValidatingReader) Read() (RawSeg, error) {
 				r.rawSeg.Elems,
 				RawSegElem{
 					// while (element) index in schema starts with 1, it actually refers to the first element
-					// AFTER the seg name element, thus we use can i as ElemIndex directly.
+					// AFTER the seg name element, thus we can use i as ElemIndex directly.
 					ElemIndex: i,
 					// comp_index always starts with 1
 					CompIndex: 1,

extensions/omniv21/fileformat/edi/seg.go

@@ -77,7 +77,7 @@ func (d *SegDecl) maxOccurs() int {
 		// for majority cases, segments have min=1, max=1, so default nil to 1
 		return 1
 	case *d.Max < 0:
-		// typically schema writer uses -1 to indicate infinite; practically max int is good enough :)
+		// typically, schema writer uses -1 to indicate infinite; practically max int is good enough :)
 		return maths.MaxIntValue
 	default:
 		return *d.Max
@@ -87,7 +87,7 @@ func (d *SegDecl) maxOccurs() int {
 func (d *SegDecl) matchSegName(segName string) bool {
 	switch d.isGroup() {
 	case true:
-		// Group (or so called loop) itself doesn't have a segment name in EDI file (we do assign a
+		// Group (or so-called loop) itself doesn't have a segment name in EDI file (we do assign a
 		// name to it for xpath query reference, but that name isn't a segment name per se). A
 		// group/loop's first non-group child, recursively if necessary, can be used as the group's
 		// identifying segment name, per EDI standard. Meaning if a group's first non-group child's

extensions/omniv21/fileformat/fileformat.go

@@ -35,7 +35,7 @@ type FormatReader interface {
 	// For certain errors (like EOF or corruption) there is no point to keep on trying; while others
 	// can be safely ignored.
 	IsContinuableError(err error) bool
-	// FormatReader must be able to format an error by providing context information (such as input
+	// CtxAwareErr allows FormatReader to format an error by providing context information (such as input
 	// file name and (approx.) error location, such as line number)
 	errs.CtxAwareErr
 }

extensions/omniv21/fileformat/fixedlength/reader.go

@@ -98,7 +98,7 @@ func (r *reader) readByHeaderFooterEnvelope() (*idr.Node, error) {
 		return nil, ErrInvalidEnvelope(r.fmtErrStr("incomplete envelope: %s", err.Error()))
 	}
 	for ; r.envelopeIndex < len(r.decl.Envelopes); r.envelopeIndex++ {
-		// regex's are already validated
+		// regex is already validated
 		headerRegex, _ := caches.GetRegex(r.decl.Envelopes[r.envelopeIndex].ByHeaderFooter.Header)
 		if headerRegex.Match(line) {
 			break

extensions/omniv21/fileformat/json/reader.go

@@ -14,7 +14,7 @@ type ErrNodeReadingFailed string
 
 func (e ErrNodeReadingFailed) Error() string { return string(e) }
 
-// IsErrNodeReadingFailed checks if an err is of ErrNodeReadingFailed type.
+// IsErrNodeReadingFailed checks if the `err` is of ErrNodeReadingFailed type.
 func IsErrNodeReadingFailed(err error) bool {
 	switch err.(type) {
 	case ErrNodeReadingFailed:

extensions/omniv21/fileformat/xml/reader.go

@@ -14,7 +14,7 @@ type ErrNodeReadingFailed string
 
 func (e ErrNodeReadingFailed) Error() string { return string(e) }
 
-// IsErrNodeReadingFailed checks if an err is of ErrNodeReadingFailed type.
+// IsErrNodeReadingFailed checks if the `err` is of ErrNodeReadingFailed type.
 func IsErrNodeReadingFailed(err error) bool {
 	switch err.(type) {
 	case ErrNodeReadingFailed:

extensions/omniv21/transform/decl.go

@@ -91,11 +91,11 @@ type Decl struct {
 	Object map[string]*Decl `json:"object,omitempty"`
 	// Array specifies the input element is an array.
 	Array []*Decl `json:"array,omitempty"`
-	// ResultType specifies the desired output type of an element.
+	// ResultType specifies the desired output type of element.
 	ResultType *resultType `json:"type,omitempty"`
 	// NoTrim specifies space trimming in string value of the output element.
 	NoTrim bool `json:"no_trim,omitempty"`
-	// KeepEmptyOrNull specifies whether or not keep an empty/null output or not.
+	// KeepEmptyOrNull specifies whether to keep an empty/null output or not.
 	KeepEmptyOrNull bool `json:"keep_empty_or_null,omitempty"`
 
 	// Internal fields are computed at schema loading time.

extensions/omniv21/transform/parse.go

@@ -16,7 +16,7 @@ type parseCtx struct {
 	transformCtx          *transformctx.Ctx
 	customFuncs           customfuncs.CustomFuncs
 	customParseFuncs      CustomParseFuncs // Deprecated.
-	disableTransformCache bool             // by default we have caching on. only in some tests we turn caching off.
+	disableTransformCache bool             // by default, we have caching on. only in some tests we turn caching off.
 	transformCache        map[string]interface{}
 }
 

extensions/omniv21/transform/validate.go

@@ -88,7 +88,7 @@ func (ctx *validateCtx) validateXPath(fqdn string, decl *Decl, templateRefStack
 		return fmt.Errorf("'%s' cannot set both 'xpath' and 'xpath_dynamic' at the same time", fqdn)
 	}
 	// unlike `xpath` which is a constant string, `xpath_dynamic` value comes from the computation of
-	// regular decl and it can be of a const/field/custom_func/template/external, so we need to parse
+	// regular decl, and it can be of a const/field/custom_func/template/external, so we need to parse
 	// and validate the decl as well.
 	if decl.XPathDynamic != nil {
 		var err error

header/header.go

@@ -9,8 +9,8 @@ import (
 
 // ParserSettings defines the common header (and its JSON format) for all schemas across all schema handlers.
 // It contains vital information about which handler a schema wants to use, and what file format the input
-// stream is of (e.g. fixed-length txt, CSV/TSV, XML, JSON, EDI, etc). Also optionally, it specifies the
-// expected the encoding scheme for the input streams this schema is used for.
+// stream is of (e.g. fixed-length txt, CSV/TSV, XML, JSON, EDI, etc). Optionally, it specifies the expected
+// encoding scheme for the input streams this schema is used for.
 type ParserSettings struct {
 	Version        string  `json:"version,omitempty"`
 	FileFormatType string  `json:"file_format_type,omitempty"`

idr/jsonnode.go

@@ -4,7 +4,7 @@ import (
 	"fmt"
 )
 
-// JSONType is the type of a JSON-specific Node.
+// JSONType is the type of JSON-specific Node.
 // Note multiple JSONType can be bit-wise OR'ed together.
 type JSONType uint
 

idr/jsonreader.go

@@ -21,6 +21,24 @@ type JSONStreamReader struct {
 }
 
 // streamCandidateCheck checks if sp.cur is a potential stream candidate.
+// Note even if sp.cur is marked, it only means it could potentially be the
+// stream candidate, not guaranteed. E.g., say have a JSON doc looks like this:
+// {
+//    "x": {
+//       "a": 5
+//    },
+//    "y": {
+//       "a": 2
+//    }
+// }
+// And our stream xpath is "/*/a[. < 4]".
+// When the reader first encounters "x/a" node, it has the potential to be the
+// stream candidate since it matches "/*/a". But we haven't finished processing
+// the entire node, thus no way we know if the value of "a" would match the filter
+// or not. So we have to mark it as a potential stream candidate and will let
+// wrapUpCurAndTargetCheck to do the final check when the entire node of "/x/a"
+// is ingested and processed, in which case, "/x/a" will be not be considered
+// as stream target, but later "/x/b" will be.
 func (sp *JSONStreamReader) streamCandidateCheck() {
 	if sp.xpathExpr != nil && sp.stream == nil && MatchAny(sp.root, sp.xpathExpr) {
 		sp.stream = sp.cur
@@ -33,7 +51,7 @@ func (sp *JSONStreamReader) streamCandidateCheck() {
 // - Either we don't have a stream filter xpath or the stream filter xpath matches.
 func (sp *JSONStreamReader) wrapUpCurAndTargetCheck() *Node {
 	cur := sp.cur
-	// No matter what outcome the wrapUpCurAndTargetCheck() is, the current node is done and
+	// No matter what outcome the wrapUpCurAndTargetCheck() is, the current node is done, and
 	// we need to adjust sp.cur to its parent.
 	sp.cur = sp.cur.Parent
 	// Only do stream target check if the finished cur node is the stream candidate
@@ -78,7 +96,7 @@ func (sp *JSONStreamReader) addTextChild(tok interface{}) {
 	}
 	child := CreateJSONNode(TextNode, data, jtype)
 	AddChild(sp.cur, child)
-	// If the child being added is a value node, there won't be anything else
+	// Since the child being added is a value node, there won't be anything else
 	// added below it, so no need to advance sp.cur to child.
 }
 
@@ -100,7 +118,7 @@ func (sp *JSONStreamReader) parseDelim(tok json.Delim) *Node {
 			// already done the check when the property itself is processed.
 			sp.cur.FormatSpecific = JSONTypeOf(sp.cur) | JSONObj
 		case IsJSONRoot(sp.cur):
-			// if we see "{" directly on root, make the root node a obj type container
+			// if we see "{" directly on root, make the root node an obj type container
 			// and do stream candidate check.
 			sp.cur.FormatSpecific = JSONTypeOf(sp.cur) | JSONObj
 			sp.streamCandidateCheck()
@@ -138,7 +156,7 @@ func (sp *JSONStreamReader) parseVal(tok json.Token) *Node {
 	case IsJSONObj(sp.cur):
 		sp.addElementChild(tok.(string), JSONProp)
 		sp.streamCandidateCheck()
-	// similarly we want arr check before prop check.
+	// Similarly, we want arr check before prop check.
 	case IsJSONArr(sp.cur):
 		// if parent is an array or root, so we're adding a value directly to
 		// the array or root, by creating an anonymous element node, then the
@@ -158,7 +176,7 @@ func (sp *JSONStreamReader) parseVal(tok json.Token) *Node {
 		}
 	case IsJSONRoot(sp.cur):
 		// A value is directly setting on root. We need to do both stream candidate check
-		// as well as target check.
+		// and target check.
 		sp.streamCandidateCheck()
 		sp.addTextChild(tok)
 		ret := sp.wrapUpCurAndTargetCheck()

idr/marshal2.go

@@ -27,7 +27,7 @@ func (ctx *ctx) isChildText(n *Node) bool {
 	//    <efg/>
 	//  </abc>
 	// If isChildText is called upon node <abc>, it should ideally return false due to the sub
-	// element node <efg> presence. However due to the way XML nodes are constructed, there will
+	// element node <efg> presence. However, due to the way XML nodes are constructed, there will
 	// be "dummy" text nodes hanging around:
 	//  ElementNode ("abc")
 	//    TextNode ("\n")
@@ -56,7 +56,7 @@ func (ctx *ctx) isChildText(n *Node) bool {
 			// We've found text child node, but we need to continue probing.
 		} else if n.Type == ElementNode {
 			elemNodeFound = true
-			// We've found an unwanted element child node, don't probing.
+			// We've found an unwanted element child node, don't probe.
 			break
 		}
 		// attribute child node is ignored.
@@ -75,12 +75,12 @@ func (ctx *ctx) isChildArray(n *Node) bool {
 	// empty string.
 	//
 	// XML/EDI are a bit more complicated:
-	// There is not concept of array in XML, neither in EDI. XML, however, can simulate
+	// There is no concept of array in XML, neither in EDI. XML, however, can simulate
 	// array by having a number of child elements with the same element name. EDI has
 	// segment loop. So the primary idea is to check child element nodes' names and use
 	// their similarity to determine whether the children represent an array or not.
 	// Throwing a wrench here is that fact in XML, there are dummy text nodes sprinkled
-	// around, in between XML elements, so we need to ignore them. Also there could be
+	// around, in between XML elements, so we need to ignore them. There could also be
 	// attribute nodes in XML, need to ignore them too.
 	//
 	// One ambiguous case in XML (and similarly in EDI):

idr/node.go

@@ -7,17 +7,17 @@ import (
 	"sync/atomic"
 )
 
-// NodeType is the type of a Node in an IDR.
+// NodeType is the type of Node in an IDR.
 type NodeType uint
 
 const (
 	// DocumentNode is the type of the root Node in an IDR tree.
 	DocumentNode NodeType = iota
-	// ElementNode is the type of an element Node in an IDR tree.
+	// ElementNode is the type of element Node in an IDR tree.
 	ElementNode
-	// TextNode is the type of an text/data Node in an IDR tree.
+	// TextNode is the type of text/data Node in an IDR tree.
 	TextNode
-	// AttributeNode is the type of an attribute Node in an IDR tree.
+	// AttributeNode is the type of attribute Node in an IDR tree.
 	AttributeNode
 )
 
@@ -41,14 +41,14 @@ func (nt NodeType) String() string {
 // by the omniparser.
 // Credit: this is by and large a copy and some adaptation from
 // https://github.com/antchfx/xmlquery/blob/master/node.go. The reasons we want to have our own struct:
-// - more stability
 // - one struct to represent XML/JSON/EDI/CSV/txt/etc. Vs antchfx's work have one struct (in each repo)
 //   for each format.
 // - Node allocation recycling.
+// - more stability
 type Node struct {
 	// ID uniquely identifies a Node, whether it's newly created or recycled and reused from
 	// the node allocation cache. Previously we sometimes used a *Node's pointer address as a
-	// unique ID which isn't sufficiently unique anymore given the introduction of using
+	// unique ID which isn't sufficiently unique any more given the introduction of using
 	// sync.Pool for node allocation caching.
 	ID int64
 

idr/query.go

@@ -18,7 +18,7 @@ var (
 const (
 	// DisableXPathCache disables caching xpath compilation when MatchAll/MatchSingle
 	// are called. Useful when caller knows the xpath string isn't cache-able (such as
-	// containing unique IDs, timestamps, etc) which would otherwise cause the xpath
+	// containing unique IDs, timestamps, etc.) which would otherwise cause the xpath
 	// compilation cache grow unbounded.
 	DisableXPathCache = uint(1) << iota
 )

idr/xmlreader.go

@@ -22,6 +22,7 @@ type XMLStreamReader struct {
 }
 
 // streamCandidateCheck checks if sp.cur is a potential stream candidate.
+// See more details/explanation in JSONStreamReader.streamCandidateCheck.
 func (sp *XMLStreamReader) streamCandidateCheck() {
 	if sp.xpathExpr != nil && sp.stream == nil && MatchAny(sp.root, sp.xpathExpr) {
 		sp.stream = sp.cur
@@ -34,7 +35,7 @@ func (sp *XMLStreamReader) streamCandidateCheck() {
 // - Either we don't have a stream filter xpath or the stream filter xpath matches.
 func (sp *XMLStreamReader) wrapUpCurAndTargetCheck() *Node {
 	cur := sp.cur
-	// No matter what outcome the wrapUpCurAndTargetCheck() is, the current node is done and
+	// No matter what outcome the wrapUpCurAndTargetCheck() is, the current node is done, and
 	// we need to adjust sp.cur to its parent.
 	sp.cur = sp.cur.Parent
 	// Only do stream target check if the finished cur node is the stream candidate