Implement suggestions

Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>

main.go

@@ -127,7 +127,7 @@ This directive runs executable with arguments and put its stderr and stdout outp
 		"Absolute path links will be converted to relative links to anchor dir as well.").Regexp()
 	// TODO(bwplotka): Add cache in file?
 	linksValidateEnabled := cmd.Flag("links.validate", "If true, all links will be validated").Short('l').Bool()
-	linksValidateConfig := extflag.RegisterPathOrContent(cmd, "links.validate.config", "YAML file for skipping link check, with spec defined in github.com/bwplotka/mdox/pkg/linktransformer.Config", extflag.WithEnvSubstitution(true))
+	linksValidateConfig := extflag.RegisterPathOrContent(cmd, "links.validate.config", "YAML file for skipping link check, with spec defined in github.com/bwplotka/mdox/pkg/linktransformer.Config", extflag.WithEnvSubstitution())
 
 	cmd.Run(func(ctx context.Context, logger log.Logger) (err error) {
 		var opts []mdformatter.Option

pkg/extflag/pathorcontent.go

@@ -96,16 +96,16 @@ func (p *PathOrContent) Content() ([]byte, error) {
 }
 
 // WithRequired allows you to override default required option.
-func WithRequired(r bool) Option {
+func WithRequired() Option {
 	return func(p *PathOrContent) {
-		p.required = r
+		p.required = true
 	}
 }
 
 // WithRequired allows you to override default envSubstitution option.
-func WithEnvSubstitution(e bool) Option {
+func WithEnvSubstitution() Option {
 	return func(p *PathOrContent) {
-		p.envSubstitution = e
+		p.envSubstitution = true
 	}
 }
 

pkg/mdformatter/linktransformer/link.go

@@ -253,31 +253,11 @@ func (v *validator) visit(filepath string, dest string) {
 	}
 	validator := v.validateConfig.GetValidatorForURL(dest)
 	if validator != nil {
-		matched, err := validator.IsValid(dest)
+		matched, err := validator.IsValid(k, v)
 		if matched && err == nil {
 			return
 		}
 	}
-
-	// Result will be in future.
-	v.destFutures[k].resultFn = func() error { return v.remoteLinks[dest] }
-	v.rMu.RLock()
-	if _, ok := v.remoteLinks[dest]; ok {
-		v.rMu.RUnlock()
-		return
-	}
-	v.rMu.RUnlock()
-
-	v.rMu.Lock()
-	defer v.rMu.Unlock()
-	// We need to check again here to avoid race.
-	if _, ok := v.remoteLinks[dest]; ok {
-		return
-	}
-
-	if err := v.c.Visit(dest); err != nil {
-		v.remoteLinks[dest] = errors.Wrapf(err, "remote link %v", dest)
-	}
 }
 
 type localLinksCache map[string]*[]string

pkg/mdformatter/linktransformer/link_test.go

@@ -239,7 +239,7 @@ func TestValidator_TransformDestination(t *testing.T) {
 		testutil.Equals(t, 0, len(diff), diff.String())
 
 		_, err = mdformatter.IsFormatted(context.TODO(), logger, []string{testFile}, mdformatter.WithLinkTransformer(
-			MustNewValidator(logger, []byte("version: 1\n\nvalidators:\n  - regex: 'github'\n    type: 'roundtrip'\n"), anchorDir),
+			MustNewValidator(logger, []byte("version: 1\n\nvalidators:\n  - regex: 'bwplotka'\n    type: 'ignore'\n"), anchorDir),
 		))
 		testutil.Ok(t, err)
 	})

pkg/mdformatter/linktransformer/validator.go

@@ -6,18 +6,20 @@ package linktransformer
 import (
 	"strconv"
 	"strings"
+
+	"github.com/pkg/errors"
 )
 
 type Validator interface {
-	IsValid(URL string) (bool, error)
+	IsValid(k futureKey, r *validator) (bool, error)
 }
 
 // GitHubValidator.IsValid skips visiting all github issue/PR links.
-func (v GitHubValidator) IsValid(URL string) (bool, error) {
+func (v GitHubValidator) IsValid(k futureKey, r *validator) (bool, error) {
 	// Find rightmost index of match i.e, where regex match ends.
 	// This will be where issue/PR number starts. Split incase of section link and convert to int.
-	rightmostIndex := v._regex.FindStringIndex(URL)
-	stringNum := strings.Split(URL[rightmostIndex[1]:], "#")
+	rightmostIndex := v._regex.FindStringIndex(k.dest)
+	stringNum := strings.Split(k.dest[rightmostIndex[1]:], "#")
 	num, err := strconv.Atoi(stringNum[0])
 	if err != nil {
 		return false, err
@@ -30,47 +32,58 @@ func (v GitHubValidator) IsValid(URL string) (bool, error) {
 }
 
 // RoundTripValidator.IsValid returns false if url matches, to ensure it is visited by colly.
-func (v RoundTripValidator) IsValid(URL string) (bool, error) {
-	return false, nil
+func (v RoundTripValidator) IsValid(k futureKey, r *validator) (bool, error) {
+	// Result will be in future.
+	r.destFutures[k].resultFn = func() error { return r.remoteLinks[k.dest] }
+	r.rMu.RLock()
+	if _, ok := r.remoteLinks[k.dest]; ok {
+		r.rMu.RUnlock()
+		return true, nil
+	}
+	r.rMu.RUnlock()
+
+	r.rMu.Lock()
+	defer r.rMu.Unlock()
+	// We need to check again here to avoid race.
+	if _, ok := r.remoteLinks[k.dest]; ok {
+		return true, nil
+	}
+
+	if err := r.c.Visit(k.dest); err != nil {
+		r.remoteLinks[k.dest] = errors.Wrapf(err, "remote link %v", k.dest)
+		return false, nil
+	}
+	return true, nil
 }
 
 // IgnoreValidator.IsValid returns true if matched so that link in not checked.
-func (v IgnoreValidator) IsValid(URL string) (bool, error) {
+func (v IgnoreValidator) IsValid(k futureKey, r *validator) (bool, error) {
 	return true, nil
 }
 
 // GetValidatorForURL returns correct Validator by matching URL.
 func (v Config) GetValidatorForURL(URL string) Validator {
-	var u Validator
 	for _, val := range v.Validators {
 		switch val.Type {
 		case roundtripValidator:
 			if !val.rtValidator._regex.MatchString(URL) {
 				continue
 			}
-			u = val.rtValidator
-			return u
+			return val.rtValidator
 		case githubValidator:
 			if !val.ghValidator._regex.MatchString(URL) {
 				continue
 			}
-			u = val.ghValidator
-			return u
+			return val.ghValidator
 		case ignoreValidator:
 			if !val.igValidator._regex.MatchString(URL) {
 				continue
 			}
-			u = val.igValidator
-			return u
+			return val.igValidator
 		default:
-			continue
+			panic("unexpected validator type")
 		}
 	}
-	// By default all links are ignored.
-	u = IgnoreValidator{}
 	// No config file passed, so all links must be checked.
-	if len(v.Validators) == 0 {
-		u = nil
-	}
-	return u
+	return RoundTripValidator{}
 }