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.

Sign up for GitHub

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

Fix clilog file names for link errors #31

Merged
merged 2 commits into from
May 19, 2021
Merged

Fix clilog file names for link errors #31

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dc527cb
Fix clilog file name
saswatamcode May 19, 2021
da6d797
Implement suggestions
saswatamcode May 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions pkg/mdformatter/linktransformer/link.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,14 +201,23 @@ func (v *validator) Close(ctx mdformatter.SourceContext) error {
})

merr := merrors.New()
base, err := os.Getwd()
if err != nil {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of things:

  • merr.Add already checks if err != nil so no need to check again
  • Don't wrap with Wrapf if there is no argument, use just Wrap
  • No need to say something failed - you are wrapping failure after all, so just resolve working dir would be ok
  • Are we sure we want to continue after those errors and not fail whole thing? why?

Copy link
Collaborator Author

@saswatamcode saswatamcode May 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes! Would be better to fail whole thing and return error rather than add to merr. Will implement this! Would need to keep err != nil checks in this case.

return errors.Wrap(err, "resolve working dir")
}
path, err := filepath.Rel(base, ctx.Filepath)
if err != nil {
return errors.Wrap(err, "find relative path")
}

for _, k := range keys {
f := v.destFutures[k]
if err := f.resultFn(); err != nil {
if f.cases == 1 {
merr.Add(err)
merr.Add(errors.Wrapf(err, "%v", path))
continue
}
merr.Add(errors.Wrapf(err, "(%v occurrences)", f.cases))
merr.Add(errors.Wrapf(err, "%v (%v occurrences)", path, f.cases))
}
}
return merr.Err()
Expand Down
25 changes: 18 additions & 7 deletions pkg/mdformatter/linktransformer/link_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,11 @@ func TestValidator_TransformDestination(t *testing.T) {

t.Run("check invalid local links", func(t *testing.T) {
testFile := filepath.Join(tmpDir, "repo", "docs", "test", "invalid-local-links.md")
filePath := "/repo/docs/test/invalid-local-links.md"
wdir, err := os.Getwd()
testutil.Ok(t, err)
relDirPath, err := filepath.Rel(wdir, tmpDir)
testutil.Ok(t, err)
testutil.Ok(t, ioutil.WriteFile(testFile, []byte(`# yolo

[1](.) [2](#not-yolo) [3](../test2/invalid-local-links.md) [4](../test/invalid-local-links.md#not-yolo) [5](../test/doc.md)
Expand All @@ -196,17 +201,23 @@ func TestValidator_TransformDestination(t *testing.T) {
MustNewValidator(logger, regexp.MustCompile(`^$`), anchorDir),
))
testutil.NotOk(t, err)
testutil.Equals(t, fmt.Sprintf("%v/repo/docs/test/invalid-local-links.md: 4 errors: "+
"link ../test2/invalid-local-links.md, normalized to: %v/repo/docs/test2/invalid-local-links.md: file not found; "+
"link ../test/invalid-local-links.md#not-yolo, normalized to: link %v/repo/docs/test/invalid-local-links.md#not-yolo, existing ids: [yolo]: file exists, but does not have such id; "+
"link ../test/doc.md, normalized to: %v/repo/docs/test/doc.md: file not found; "+
"link #not-yolo, normalized to: link %v/repo/docs/test/invalid-local-links.md#not-yolo, existing ids: [yolo]: file exists, but does not have such id",
tmpDir, tmpDir, tmpDir, tmpDir, tmpDir), err.Error())

testutil.Equals(t, fmt.Sprintf("%v: 4 errors: "+
"%v: link ../test2/invalid-local-links.md, normalized to: %v/repo/docs/test2/invalid-local-links.md: file not found; "+
"%v: link ../test/invalid-local-links.md#not-yolo, normalized to: link %v/repo/docs/test/invalid-local-links.md#not-yolo, existing ids: [yolo]: file exists, but does not have such id; "+
"%v: link ../test/doc.md, normalized to: %v/repo/docs/test/doc.md: file not found; "+
"%v: link #not-yolo, normalized to: link %v/repo/docs/test/invalid-local-links.md#not-yolo, existing ids: [yolo]: file exists, but does not have such id",
tmpDir+filePath, relDirPath+filePath, tmpDir, relDirPath+filePath, tmpDir, relDirPath+filePath, tmpDir, relDirPath+filePath, tmpDir), err.Error())
})

t.Run("check 404 link", func(t *testing.T) {
testFile := filepath.Join(tmpDir, "repo", "docs", "test", "invalid-link.md")
testutil.Ok(t, ioutil.WriteFile(testFile, []byte("https://bwplotka.dev/does-not-exists\n"), os.ModePerm))
filePath := "/repo/docs/test/invalid-link.md"
wdir, err := os.Getwd()
testutil.Ok(t, err)
relDirPath, err := filepath.Rel(wdir, tmpDir)
testutil.Ok(t, err)

diff, err := mdformatter.IsFormatted(context.TODO(), logger, []string{testFile})
testutil.Ok(t, err)
Expand All @@ -216,7 +227,7 @@ func TestValidator_TransformDestination(t *testing.T) {
MustNewValidator(logger, regexp.MustCompile(`^$`), anchorDir),
))
testutil.NotOk(t, err)
testutil.Equals(t, tmpDir+"/repo/docs/test/invalid-link.md: \"https://bwplotka.dev/does-not-exists\" not accessible; status code 404: Not Found", err.Error())
testutil.Equals(t, fmt.Sprintf("%v%v: %v%v: \"https://bwplotka.dev/does-not-exists\" not accessible; status code 404: Not Found", tmpDir, filePath, relDirPath, filePath), err.Error())
})

t.Run("check 404 link, ignored", func(t *testing.T) {
Expand Down