Sam Mohr (May 07 2024 at 05:36): Sam Mohr (May 07 2024 at 05:36):I don't want to be too arrogant, but it does seem that if we're not happy with the built-in Arg parser in basic-cli, we can probably delete (or deprecate) it in favor of suggesting Weaver. The only function that still needs to stay is Arg.list, which in theory could be moved to Env.args if we don't want to keep a module with only a single function in it. Any thoughts? I'm definitely aware of the bus factor here, and would be happy to transfer Weaver somewhere else if desired by the community.
Sam Mohr (May 07 2024 at 05:38):Rust does have its args function in std::env::args, so there is naming precedent here
Sam Mohr (May 07 2024 at 06:26):Created a PR in basic-cli: https://github.com/roc-lang/basic-cli/pull/204
Anton (May 07 2024 at 12:07):Not arrogant at all, what's making me hesitate is that I do want an args example in the basic-cli examples but relying on weaviate may make things difficult when going through breaking Roc changes. We'd be forced to make a new weaviate release before we can make a new basic-cli release which I would like to avoid. How do you feel about directly integrating/copying weaviate in basic-cli?
Hristo (May 07 2024 at 12:41):We'd be forced to make a new weaviate release before we can make a new basic-cli release which I would like to avoid.
+1 for preferably avoiding such kind of a dependency. I think there are some very good lessons from other language ecosystems which suggest that avoiding such dependency is probably the correct direction in this case.
Hristo (May 07 2024 at 13:04):Unless Weaver and basic-cli get properly merged together, so that there isn't a separate dependency outside the official streams? In other words weave Weaver into basic-cli (I couldn't resist the wordplay, sorry :smiley:)
Anton (May 07 2024 at 13:17):Unless Weaver and basic-cli get properly merged together,
Yeah that's what I proposed as well :p
Anton said:
... How do you feel about directly integrating/copying weaviate in basic-cli?
Hristo (May 07 2024 at 13:26):Indeed, @Anton! Apologies - I missed that bit :pray:
Anton (May 07 2024 at 13:56):All good :)
So yeah, merging seems like the way to go!
Sam Mohr (May 07 2024 at 15:24):@Anton I agree that integration is probably the way to go for the next year or two, but long term, when other platforms are being built, including other CLI platforms, then we are making it harder to code share this builder code.
It feels like module params and Task as a builtin will be the best mechanism for making code like Weaver modular, and I'm worried about losing its modularity. Do you disagree?
Anton (May 07 2024 at 16:02):I thought about this as well, the weaver repo could pull in files from the basic-cli repo and make them available in a standalone package too.
Richard Feldman (May 07 2024 at 16:02):whoa, I never thought of that!
Hristo (May 07 2024 at 16:17):Yes, this way Weaver could have arbitrarily many improvements, which will still be based on some stable "core", belonging to basic-cli. If basic-cli likes any of them, they'll be introduced to it as well. Something like Weaver being a "logical fork" (to a certain extent) of basic-cli. This is my (possibly partially or entirely incorrect) conceptual understanding of the dependencies. Basically, the directionality is what matters there - one wouldn't want for basic-cli to be dependent on external releases even though, in theory, such dependencies could be arbitrarily forked (e.g., in case the maintainer team of said dependencies does not have time to keep them up-to-date) etc, but the messiness level associated with this approach seems unnecessarily high to me .
Sam Mohr (May 11 2024 at 06:30):Can you explain what you're suggesting here? I'm not sure what you think would be brought into Weaver besides Task-related code, implying that Weaver would copy/somehow re-export a platform in the style of basic-cli? I'm not seeing that much of a benefit if that's the case.
Anton (May 11 2024 at 09:24):Can you explain what you're suggesting here?
Yeah sure, so for now the source code of the weaver package would not change. My proposal is to put those exact source files in basic-cli to avoid duplication and potential issues with breaking Roc changes. The weaver repo would basically only contain main.roc:
package [
Opt,
Base,
Cli,
ErrorFormatter,
Help,
Param,
Subcommand,
Validate,
] {}
To make a package release of weaver, the CI workflow would fetch some files from basic-cli like Base.roc, Builder.roc, ... and create the package bundle. Over time, the weaver package could contain extra features compared to basic-cli if desired.
Sam Mohr (May 11 2024 at 10:54):@Anton okay, the more I think about it, the more that seems to handle both scenarios we want to cover:
1) basic-cli users have everything built in, and can go on their merry way
2) future use of the library is safely protected by having the package published in its own repo in GitHub, and users can decide to use the library on its own if they want to use a different platform
Sam Mohr (May 11 2024 at 10:55):And this also gives us some niceties for Weaver that would otherwise take more module params than would probably be desirable:
Sam Mohr (May 11 2024 at 10:58):So the only question I have to the group is where in basic-cli should weaver go?
Args.Opt, Args.Param, etc.Weaver.Opt, Weaver.Param, etc.Opt, Param, etc.I vote the second option, which retains more personality than is present in any of the other modules, but I think it's fun to have these kind of things in a "standard library" like basic-cli
Anton (May 11 2024 at 11:13):I would personally prefer to go with Args, it's very self-explanatory. I expect users would have a hard time guessing what "Weaver" relates to, when they see it for the first time.
Sam Mohr (May 11 2024 at 11:18):Yeah, fair. There may be some issues with documentation misaligning between the basic-cli files and the package, where they'll both now say the Args module or something to that effect. I'll see if I can reword things to be either agnostic or conscious of the dual state of the library
Anton (May 11 2024 at 11:25):Sounds good, thanks :)
Sam Mohr (May 12 2024 at 07:39):I've made a commit to the weaver -> Args PR, and there are some nice benefits to usability from having the platform available, but I'm not super happy with the results.
The blocker I'm facing now is in avoiding polluting the module space for basic-cli with top-level modules like Opt and Param, I've had to make the interface worse (in my opinion) for the user. Users now have to import import pf.Args.Opt as Opt instead of import weaver.Opt for pretty much everything they use. This could be circumvented in a couple ways:
Args (or maybe a couple modules). I _really_ don't like the implication of this one from a code cleanliness standpoint (read: 4k+ line module), but imports are now clean again and basic-cli isn't organizationally polluted.For any of these three options, anything that has nested modules runs into an unfortunate problem: imports relative to the root of the package have the name of the parent module in them, so if weaver wants imports to be import weaver.Opt, but basic-cli has the file in platform/Args/Opt.roc, then we can't just symlink a file in the weaver repo, since Args.Opt will import sibling modules as import Args.Parser and not just Parser. So for us to have shared files between Weaver and basic-cli, they need to get their imports find-and-replaced on distribution and testing.
This change is doable, but seems to point to Weaver being embedded in basic-cli not being a "zero-cost abstraction", so to say. Unless there's a cleaner way to share the code, it seems much cleaner and future-proof to just have Weaver in its own repo for the time being and wait for module params for the Task management. I don't think that the concern that Weaver releases are needed for every basic-cli release/Roc language change is a big deal, since it's equally true for every other library in Roc at the moment (even important ones like roc-json).
I appreciate the learning value of this code sharing experiment, but I don't think the juice is worth the squeeze.
Sam Mohr (May 12 2024 at 07:40):I want to make sure I'm keeping my mind open to finding a good solution, so if anyone wants to take a stab, I'm happy to work together on it!
Sam Mohr (May 12 2024 at 07:56):Now, given the context for this discussion comes from my proposal to remove the Args module in favor of using Weaver, if we still the in-built Args parser to be maintained so that we aren't dependent on an external package, then maybe I can take a look at getting it working again? I don't think anyone that knows about Weaver is likely to reach for it, but I understand the value of having something built-in. However, I would still personally vote that we should remove the Args module and move Args.list to Env.args (as was done in my first commit of the PR).
Brendan Hansknecht (May 12 2024 at 10:49):Should import pf.Args.Opt as Opt and import pf.Args.Opt do the exact same thing?
Brendan Hansknecht (May 12 2024 at 10:50):Also, I think your comment on sibling module imports is a bug
Brendan Hansknecht (May 12 2024 at 10:51):the Args.Parser module should import Args.Opt with the line import Opt
Agus Zubiaga (May 12 2024 at 11:28):Not a bug unless I’m missing something. We just inherited this from Elm.
Agus Zubiaga (May 12 2024 at 11:29):Richard has some ideas for how nested modules should work, but we found some issues and determined it needed more design work.
Agus Zubiaga (May 12 2024 at 11:48):Brendan Hansknecht said:
the
Args.Parsermodule should importArgs.Optwith the lineimport Opt
We talked about doing this in the context of the changes in the proposal. The problem is that, currently, you’re able to import modules from the parent level. So what if there also was an Opt module there?
Agus Zubiaga (May 12 2024 at 11:50):We would either need to disallow this or introduce some other syntax for it. The solution wasn’t obvious so we kept the previous behavior for now.
Agus Zubiaga (May 12 2024 at 11:50):I think it will change in the future, though
Ayaz Hafiz (May 12 2024 at 16:27):Not to distract from this conversation but what are the issues with making import pf.Args.Opt the same as import pf.Args.Opt as Opt? I'm assuming import pf.Opt is the same as import pf.Opt as Opt? If there's not already an existing thread on this I'll make one.
Sam Mohr (May 12 2024 at 16:27):Yeah, let's make a separate thread. It's a good discussion to have
Ayaz Hafiz (May 12 2024 at 16:30):Users now have to import
import pf.Args.Opt as Optinstead ofimport weaver.Optfor pretty much everything they use.
This is unfortunate, but my intuition would be that it's likely not too big of an issue. I suspect that most applications will have arg parsing localized in one file, so you won't have to list this more than once.
Sam Mohr (May 12 2024 at 16:31):Yeah, it's minor, but unless the imports change how they work, it's still a downside.
Sam Mohr (May 12 2024 at 16:32):The main thing is the fact that the code can't just be symlinked from a git submodule
Sam Mohr (May 12 2024 at 16:32):Because of import differences, that I expect won't and shouldn't change
Ayaz Hafiz (May 12 2024 at 17:04):I would be inclined to suggest what you mentioned at the end, to either fork weaver into the args implementation in basic-cli or otherwise have args in basic-cli be its own implementation. there is a large value to basic-cli having no dependencies, at least while the language is young. There is also a large value in having basic-cli be "batteries included", so that users can be reasonably confident in pulling in a de-facto arg parsing library without having to audit ecosystem options. I think having weaver pull in files from basic-cli would end up a fork sooner or later, as the needs of either diverge.
Ayaz Hafiz (May 12 2024 at 17:06):If Roc was more mature/we didn't expect a lot of ecosystem churn it may be better to have basic-cli explicitly depend on weaver and wrap Args around it.
Sam Mohr (May 12 2024 at 17:07):Yeah, that's true. In which case, even if it leads to duplication, I'll go ahead and finish cleaning up this Args update PR that copies Weaver's code in without the branding. As I add features to the package, I can port them to basic-cli. At some point we can decide to merge them if it seems like a good idea in the future
Sam Mohr (May 12 2024 at 17:07):Sound good to you?
Ayaz Hafiz (May 12 2024 at 17:10):yeah that's what i'd be inclined to do. but definitely be sure to do what's best for you!
Sam Mohr (May 13 2024 at 08:48):@Anton when you get a chance, please re-check the PR, it's ready for review
Anton (May 13 2024 at 08:53):Will do, thanks @Sam Mohr :)
Last updated: Jul 06 2025 at 12:14 UTC