added support for literal paths in requires #169
Conversation
| 4. The last and most complex form imports `first` and `rest` | ||
| functions from the `wisp.sequence` module, although it also | ||
| renames them and there for makes available under different | ||
| `car` and `cdr` names. |
There was a problem hiding this comment.
Fixed indent here.
| (join \/ | ||
| (concat [\.] | ||
| (repeat (dec (count from)) "..") | ||
| to)))))))) |
There was a problem hiding this comment.
Indented let (as part of adding an if string? check) and swapped branches of if relative?
| @@ -1,5 +1,5 @@ | |||
| (ns wisp.test.ast | |||
| (:require [wisp.test.util :refe [is thrown?]] | |||
| (:require [wisp.test.util :refer [is thrown?]] | |||
There was a problem hiding this comment.
…Apparently importing macros does nothing after all?
There was a problem hiding this comment.
Yeah it will import all macros regardless. Intention was to only import ones been referred, but that never made it.
| (pr-str '~form) "\n" | ||
| "expected: " | ||
| (pr-str '~expected) "\n" |
There was a problem hiding this comment.
This way makes it easier to compare long literals.
| wisp.evaluate(` ; this evaluates the script | ||
| (alert "Hello world!") | ||
| `); | ||
| </script> |
|
@Gozala what do you think about giving @LeXofLeviafan commit access to this repository? These contributions are outside my ability to make a judgement call on. |
I don't mind giving @LeXofLeviafan commit access. That said, I do think someone should make judgement calls or this will serve no one at all. I would personally be much more comfortable if you were to make those calls given that you've taken a lead as opposed to someone new like @LeXofLeviafan (no offense) Turns out I am unable to grant anyone admin rights on repos under my github name, so I would like to propose to take this as an opportunity and transfer this repo to a new github org where I can grant @chr15m full admin rights and subsequently power to give write access to whoever it would make sense. As of proposed changes, I do not believe I am in a position to make any calls anymore as I have not committed to this project in years. All I can say is that it not aligned with an original vision, I intentionally was trying to fall on the side of simplicity over flexibility and that was a reason why only subset of module import syntax from Clojure was ever implemented. But then again, I do not vision at a time matters all that much today. @LeXofLeviafan I'm actually curios what use cases does this new import syntax address that were not possible before ? |
|
@Gozala I agree that it's preferable to have centralised decision-making on which commits make it into master, as well as that it's better to leave it in to someone more familiar with the project. As for your question… Before I decided to minimise runtime dependencies of my mreframe project I tried to write it in Wisp, and when I started to write tests I spent an awful lot of effort trying to import the main sources correctly (as well as making subpackages work together); and even though I ended up restructuring the project, it still took some weird workarounds to make it work (what actually did work) due to a surprising amount of quirks in the import system. |
I do not have enough context here to provide meaningful feedback, but looking at your test/atom.coffee module I would have expected following to Just Work™️: (ns mreframe.test.atom
"tests form atom"
(:require
[ospec :as o]
[mreframe.src.atom :refer [deref resetVals reset swap swapVals compareAndSet atom]]
[mreframe.src.reagent :as r]))If it did not that sounds like a bug. If it was not clear that above was a way to go than probably docs on importing could be improved to clarify. |
|
Like I said, the current (flat) package structure with nested module implementations came after quite a few design changes; originally I was trying to get closer to the package names one would import in Clojure (for sake of “more compatible” code). That caused a lot of problems with relative imports detection in Wisp… which could all be easily avoided if I had the option to specify those paths directly. The worst issue was that |
I'm sorry it did not worked. I understand those could have been addressed with relative imports but then again it was explicit choice not to support that. That is not to say that decision needs to be maintained going forward, just trying to clarify that it was not an accident but by a design.
You could also choose to not make I am sorry @LeXofLeviafan wisp did not lived up to your expectations. And I am grateful that you even took time to try to fix it so it would meet your needs. That said, I am still not convinced that importing via relative path or ability to diverge namespaces from paths is an improvement. However I'm not the authority nor I have maintained this repository for years so my opinions are irrelevant, at best context that lead to specific decisions might help guide new ones but even that is probably a stretch. |
As the original designer of Wisp I think your input is super valuable. Currently there is no authority on this codebase (certainly not myself) and so it will have to be "rough consensus and running code" as they say. For this particular PR the running code is nicely satisfied but I don't think "rough consensus" is quite there yet. |
That makes it an outright limitation of the language, which is not something that even should exist in it; placing source root in a subdirectory is a legitimate option, and making it unavailable just because something doesn't work in the language compiler (as opposed to fixing the problem itself) sounds like a bad idea to me. |
I've been fiddling with a project and came to conclusion that in some cases, allowing for custom require paths in Wisp would be quite helpful (or rather, not having such a fallback option can get seriously frustrating at times).
The idea is based on shadow-cljs implementation of JS imports.