Prevent mismatches on members named md #2526
Conversation
| if (url.endsWith(".md")) node.setUrl { | ||
| val x = new java.net.URI(url.toString) | ||
| val path = x.getPath() | ||
| val method = path.substring(path.lastIndexOf('/')+1).dropRight(3); |
There was a problem hiding this comment.
This logic here is the tricky bit. You have two alternatives:
- You're linking from to a class or some other kind that exists in source:
[md](my.package.md) - You're linking some other file that is not a class/object:
[md](../path/to/my/file.md)or[md](http://path.to/file.md)
You want to make sure that both of these are legal. As it stands now - the check below is a bit too simple, one reason being: I don't have / in (99.9% of) my URIs to classes.
There was a problem hiding this comment.
Another thing to consider - since you now have ctx you could do
ctx.error(s"""Ambiguous reference to "$url"""", pos)to show the user the ambiguity.
| (new NodeVisitor( | ||
| new VisitHandler(classOf[Link], new Visitor[Link] with MemberLookup { | ||
| override def visit(node: Link): Unit = { | ||
| val url = node.getUrl | ||
| if (url.endsWith(".md")) node.setUrl { | ||
| val x = new java.net.URI(url.toString) |
There was a problem hiding this comment.
For this variable name, you can do better thanx :)
There was a problem hiding this comment.
Yeah Sorry. I would process this with the other changes :)
| val path = x.getPath() | ||
| val method = path.substring(path.lastIndexOf('/')+1).dropRight(3); | ||
| val method1 = method.toTypeName | ||
| if ((url.endsWith(".md")) && (ctx.getClassIfDefined(method1).exists == false)) node.setUrl{ |
There was a problem hiding this comment.
ctx.getClassIfDefined(method1).exists == false is the same as: !ctx.getClassIfDefined(method1).exists. Please use the latter :)
There was a problem hiding this comment.
Ah sorry, that was more out of despair that by some unearthly force, it would work. Sadly it didn't :/
|
Could you post a gist of the stacktrace from the assertion? |
|
@felixmulder Yeah sure, sorry for the late reply but here goes: I have no idea why the tests on CI passed though... |
|
The rest of the stack is pretty normal, just usual stuff like |
| url.subSequence(0, url.lastIndexOf('.')).append(".html") | ||
|
|
||
| if (url.endsWith(".md")) { | ||
| if (url.subSequence(0, url.lastIndexOf('.')).indexOf("/") <0) { |
| if (url.endsWith(".md")) { | ||
| if (url.subSequence(0, url.lastIndexOf('.')).indexOf("/") <0) { | ||
| val method = url.subSequence(0, url.lastIndexOf('.')).toString.toTypeName | ||
| if (ctx.getClassIfDefined(method).exists) node.setUrl{ |
There was a problem hiding this comment.
space after node.setUrl
| } else { | ||
| ctx.error(s"""Ambiguous reference to "$url"""") | ||
| } | ||
| } else node.setUrl{ |
There was a problem hiding this comment.
Space after node.setUrl
There was a problem hiding this comment.
Thanks! Sorry for being late! Didn't catch your comment amongst all the action going on here :)
|
Thanks! |
|
Merging this broke |
This reverts commit 2127e9f.
Revert "Prevent mismatches on members named md (#2526)"
|
Um. I don't know why the tests passed. It has a java:lang exception error still :/ |
|
It doesn't run |
|
Still, I can't seem to figure out what's causing the exception. Any ideas? |
|
The exception suggests that something is touched outside of its allowed run. I'm sorry, right now we're in crunch mode, I'll get back to this tomorrow or the day after. |
|
No problem at all. I'm the one who should be sorry for causing the extra trouble. Really sorry! |
Intended as a fix for #1944. Tests are failing because of an
AssertionErrorwhich, after hours of desperation, I can't seem to find out. Please do suggest where the error might be. Many thanks to @felixmulder for helping me out with however basic my questions may be. Thanks for your time and patience Felix!