Comments (10)
Yeah, the issue of derives came from several people several times. I'd definitely love to do it using derives, there's even an issue still open: Kixunil/configure_me#3
I currently see these obstacles around man page generation and other tooling (there's experimental debconf
script generator already and I'd love to implement shell completions too):
- Man page generation was previously a part of the build script, but now it's moved to a separate tool to not mix up compilation with documenting. I was against this design (let people use cargo only, no makefiles) for some time, but eventually I started to like it more (separation of concerns).
- If I wanted to provide man page & co for derives, I'd either:
- have to access files from within proc macros, this
- goes against the design above
- is extremely uncommon in Rust ecosystem (didn't see it anywhere at all)
- might get impossible if syscalls from proc macros get disabled (I've read about some plans to do it)
- scan the whole source tree from external tools, parse it and find all occurrences of my derive. I like this approach a bit, but see other problems with it:
- Implementing seems to be very complex. I think I'd have to rely on someone else doing bulk of work, as I don't have too much time to implement such big things myself. According to dtolnay/proc-macro2#213 this feature doesn't exist yet.
- obviously, when Rust adds a new syntax, the parsing instantly breaks, this doesn't happen with toml. To be fair,
syn
crate seems to be pretty up to date with the new stuff, so maybe not too hard to keep it up to date - what about other macros? If someone generates the struct with another macro, then the parser can't see it. Expanding the code doesn't help because it'd also expand my derive, losing the structural information.
- I'm not sure how it'd work with interface files, which by definition have to be language-independent file format. I'm not pessimistic here, though.
- scanning the whole tree might be slow in case of big projects (not sure).
- do hacks like requiring people to have the struct in a single file that doesn't contain anything else (besides
use ...;
, I guess), this would be fairly easy, but lead to people being confused why man page generation doesn't work or even forgetting about it completely if they didn't attempt to generate man page at all. Then someone else would find it impossible to generate man page. - another hack would be to embed man page to the code and then the user could make the program itself output the man page. But what about other tools? I don't want to bake debconf script into every program...
- have to access files from within proc macros, this
I'm quite sad about this situation as I'd love to see less boilerplate (build.rs, which fortunately is currently only 3 lines), and proper IDE support. But considering the issues above, using toml seems to be a more useful trade-off to me. I hope you can see the reasons now. If you have a solution, I definitely want to know about it.
Regarding reducing the amount of generated code, what changes would you like to see? I guess I could move a bit more stuff into shared library, but I don't see how to achieve significant reduction. Most of the complexity comes from making sure that each field has a strong rustic type and well-understandable, human-readable error messages. I'm a huge fan of strong types and they were one of the primary design goals (clap had only stringly API when I started writing configure_me
). Reducing the code would also make the generator simpler, so I'm definitely interested in improvements in this area.
from lnp-node.
@Kixunil Let's do it! I am persuaded. Let's change Clap in wired
(and the rest of future daemons will follow) with configure_me. I will still ask to leave Clap in lnp-cli though...
Will much appreciate a PR!
from lnp-node.
Great, I plan to look t it soon. Having clap
in lnp-cli
for now is reasonable. Keeping track of separate dependencies might not be entirely great, so it may motivate me to add the necessary features to configure_me
sooner. :)
from lnp-node.
I think we can ignore connect for now and bring it back later when configure_me will support arrays from cli args
from lnp-node.
I implemented it. Was pretty easy, but required better support in rust-lnpbp
. Once that is merged, I think we're pretty much done with this issue.
from lnp-node.
Thanks for suggestion! I've been seeing configure_me
in electrs and was impressed by it; even more impressive that it's your work!
What I'd like to consider before adopting it. The current model that we follow — to adopt command-line options + environment using (yet unreleased) Clap 3.0 with custom derive — you may see how it works in https://github.com/LNP-BP/lnp-node/blob/aa20f431fd5f6753049eb45011b1c04b1b048daa/src/wired/config.rs#L23-L54: quite rust-native way, without pre-build code generation.
However, with this, we have config file yet uncovered: https://github.com/LNP-BP/lnp-node/blob/aa20f431fd5f6753049eb45011b1c04b1b048daa/src/bin/wired.rs#L26
While it would be good to cover this via configure_me
, but it would be great if we will be able to maintain the same macro-derive style of Clap 3.0 as in the given sample file + reduce the amount generated code at pre-built step.
In general, I did differentiate Opts
concept from Config
: Opts are stuff that is read from Env/Cli args, while config is the thing that is read from config file modified by opts at launch time. My point in doing this was the following: we need config structure since not all of the parameters can be specified via environment and command-line arguments. Thus we need a config file and default set of configuration.
May you consider the next version of config_me
with this information?
from lnp-node.
So I have something that passes typecheck. I have some linker errors so I can't test it yet. But anyway, there's one more issue: connect
argument isn't implemented because configure_me
can't accept arrays from command line and env vars yet. (in other words, it can accept them from config thanks to serde) Should I:
- Go ahead and ignore
connect
, since it isn't used anyway - Disable arguments and env vars for the time being, only config file will be used for the time being
- Wait until arrays (multiple repeated arguments?) are implemented in
configure_me
?
from lnp-node.
When writing Kixunil/configure_me#38 I realized there's a quite easy workaround for multiple arguments - by making a wrapper implementing ParseArg
by inserting and having a custom merge function. The effect of that would be that instead of replacing file configuration with arguments, it'd only extend it. It'd also merge all instances of connect if given in different files, without ability to remove them.
If you like this approach, I can write it.
from lnp-node.
It seems fine, however not sure what did you mean by "instead of replacing file configuration with arguments, it'd only extend it". Does it mean that current version can use only config file or args, and not both of them? Even if yes, for sure it would be better to move to merge procedure
from lnp-node.
It can, the difference is what happens if an option is set in config file and arg. Normally arg will just overwrite whatever is int the file, but with custom merge_fn
, the function defines the behavior. So for instance, we can write:
fn merge_vec(mut a: Vec<String>, b: Vec<String>) -> Vec<String> {
a.extend(b);
a
}
And this will cause the vecs to merge. This is handy in some scenarios (e.g. btc-rpc-proxy
can have different users specified in different files), but makes it unable to use argument to remove an existing entry from the collection.
from lnp-node.
Related Issues (20)
- `lnp-cli create` hangs after channel creation HOT 2
- a call to `connect` hangs and peerd logs "invalid nodeid" HOT 10
- channel data inconsistency between peers HOT 5
- asset identifier confusion HOT 1
- node receiving a connection does not know initiator's nodeid
- Update configure_me to 0.4
- Failed to build demo Docker image HOT 1
- No matching package named `lnp-core` found when building docker image HOT 4
- Cargo test command failing HOT 2
- Use configure_me for storing persistent configuration options
- Use `ln-types` crate? HOT 8
- Tracking: full LN interoperability
- Tracking: node operations
- lnp-node address not have regtest prefix 'bcrt' HOT 2
- Connection handshake failure for TCP connections HOT 4
- Panic starting node 'Unable to open key file' HOT 2
- Use AddressCompat instead of rust-bitcoin Address to display address
- Bolt Review + Universal Invoice + Bifrost HOT 1
- private or changing IP address considerations HOT 1
- Unknown Transport Protocol
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from lnp-node.