florence / cover

a code coverage tool for racket
MIT License
38 stars 7 forks source link

Fixes relative links in HTML output #113

Closed SuzanneSoy closed 8 years ago

SuzanneSoy commented 8 years ago

When the destination directory has more than one path component, e.g. raco cover -d foo/coverage file.rkt, the URLs were of the form coverage/coverage/file.html . Also, when the -d option was a relative path starting with ./, e.g. ./foo/coverage, the URL in the links was an absolute path, like /home/user/project/foo/coverage/file.html, which doesn't work if the generated HTML is put online.

This patch fixes both cases (foo/coverage, and ./foo/coverage, I also tested with just ".", it works).

florence commented 8 years ago

How does this now behave if you, from the top of your share/pkgs directory, do:

raco cover -c pict -c compiler ?

SuzanneSoy commented 8 years ago

@florence It hogged up all of my memory, and took an awful amount of time (both in the original and the patched versions), but diff -r tells me the result "coverage/" folders are identical (and the links were already relative in this case).

Was there something specific I should have looked for?

By the way, my installation is running on NixOS, which does some funny things to the system paths (/usr etc.), so you might want to test yourself too, if there's something specific to the system folders happening.

florence commented 8 years ago

Looking into things, this actually regresses af35a4dd1a7403a18

raco cover -p pict from a directory not a parent of my extra-package directory generate this html page: screen shot 2016-02-03 at 12 15 37 pm

The rendered links in the index should be relative to the least common relative directory of all files in the output, NOT relative the current directory.

florence commented 8 years ago

(I should really have tests for this...)

SuzanneSoy commented 8 years ago

@florence Unless I didn't see something when I changed the code, I only touched the href part of the links:

The k / name should be untouched by my change, so the visible ../../.. is another issue I think.

SuzanneSoy commented 8 years ago

@florence I agree about tests, although checking for broken links might be a bit difficult… Unless you mean checking against the HTML when it is still in s-exp form.

florence commented 8 years ago

Ok I must have broken this at some other point and not noticed.

(The proper tests would be to read the files back in and mung those in sexp form)

florence commented 8 years ago

Merged, thanks!