[#244] Symlink scanner #262
Conversation
|
I have created this draft PR to discuss a possible implementation for the symlink scanner and see possible problems. A couple of details observed for the moment:
|
| -- | Way to parse a file. | ||
| type ScanAction = CanonicalPath -> IO (FileInfo, [ScanError 'Parse]) | ||
|
|
||
| -- | All supported ways to parse a file. | ||
| type FormatsSupport = Extension -> Maybe ScanAction | ||
| type FileSupport = CanonicalPath -> IO $ Maybe ScanAction | ||
|
|
There was a problem hiding this comment.
Instead of determining which scanner to use just from the file extension, I changed it to a more general check. Another option is to put the symlink scanner in a 'different level' that is above extension check: if it is a symlink, then symlink scanner, else get scanner from extension.
There was a problem hiding this comment.
How about the type Bool -> Extension -> Maybe ScanAction, where the first bool denotes isLink?
There was a problem hiding this comment.
I like it. It is less general and can be better understood 👍
| - reference (relative) : | ||
| - text: "" | ||
| - link: dir/b.md | ||
| - anchor: - |
There was a problem hiding this comment.
This is how a symlink reference looks like by using the plain system that we already have. It can be improved to show in some way that s.md is a symlink that points to dir/b.md.
aeqz
force-pushed
the
aeqz/#242-no-scan-symlinks-as-md
branch
2 times, most recently
from
066da26 to
1431587
Compare
aeqz
force-pushed
the
aeqz/#244-symlink-scanner
branch
from
d9f2a01 to
8cf4d34
Compare
aeqz
force-pushed
the
aeqz/#242-no-scan-symlinks-as-md
branch
2 times, most recently
from
e45ebd8 to
fef5153
Compare
aeqz
force-pushed
the
aeqz/#244-symlink-scanner
branch
from
8cf4d34 to
e6ae598
Compare
There was a problem hiding this comment.
Looks good, I like how the new scanner addition is handled.
Leaving a few comments. And also would eventually like to see how it looks after rebase over #263.
aeqz
force-pushed
the
aeqz/#244-symlink-scanner
branch
from
e6ae598 to
84fcc86
Compare
aeqz
force-pushed
the
aeqz/#244-symlink-scanner
branch
from
ffbf20f to
63c9e28
Compare
| ] | ||
| firstFileSupport :: [FileSupport] -> FileSupport | ||
| firstFileSupport fs isSymlink = | ||
| safeHead . catMaybes <$> traverse ($ isSymlink) fs |
There was a problem hiding this comment.
Perhaps instead of taking the first FileSupport that handles the file, we can join all the results of scanning the file with all the FileSupports that can handle it, although currently we only have Markdown and Symlink scanners, which are exclusive.
There was a problem hiding this comment.
That farsighted фтв actually makes sense. However I think let's leave it to change to people who will add other scanners.
aeqz
force-pushed
the
aeqz/#244-symlink-scanner
branch
from
63c9e28 to
643adf9
Compare
Problem: As GitHub and GitLab do not render symlinks as the file they point to, we are considering to implement a new scanner for symlinks that verifies them up to some extent. Solution: A scanner that validates the reference from a symlink has been implemented in the same style as the markdown scanner.
aeqz
force-pushed
the
aeqz/#244-symlink-scanner
branch
from
643adf9 to
9421c42
Compare
Description
Problem: as GitHub and GitLab do not render symlinks as the file they point to, and we have disabled the markdown scanner to process them in #261, we are also considering to implement a new scanner for symlinks that verifies them up to some extent.
Solution: in this proposal, a scanner that validates the reference from a symlink has been implemented in the same style as the markdown scanner.
Related issue(s)
Fixes #244
✅ Checklist for your Pull Request
Ideally a PR has all of the checkmarks set.
If something in this list is irrelevant to your PR, you should still set this
checkmark indicating that you are sure it is dealt with (be that by irrelevance).
Related changes (conditional)
Tests
silently reappearing again.
Documentation
Public contracts
of Public Contracts policy.
and
Stylistic guide (mandatory)