Closed Antosser closed 2 months ago
So it would track the current problem only? Wouldn't that be similar to what was proposed in https://github.com/rust-practice/cargo-leet/pull/84#issuecomment-2181825418 to use a struct to track the active problem and any other settings we want to save?
Oh, totally. Sorry, forgot to attribute that to you. What are the advantages of should_use_problem_number_in_name
?
And I'm thinking toml
would suit it better. RON
is less known and only gives an advantage if you have a complex structure
Sure toml, is fine. I was mostly trying to see if there were differences. should_use_problem_number_in_name
allows the user to decide if the file created should include the problem number. I like to have the number some of the others didn't. So it's optional. But the default is no number.
Alright. Are you good with Leet.toml
as the config file name and with the features approach?
I'm not super clear on what you mean by features. And for the name can we use .leet.toml
instead?
Would it be possible for you to create a sample version of the file so I can get a better idea of what you mean by features?
I meant to literally use Rust features to isolate problems and run tests individually without having to compile every single one
Ah I see, I don't fully understand how it would work but I'm ok with seeing what it looks like. But wouldn't it be easier to just comment them out from lib.rs or remove the lines in lib.rs?
But yes in general I'm ok with the general structure of the plan.
Good idea. The CLI could add the //
before testing and remove them after it's done, so it won't interfere with git
Sure that doesn't sound like a problem.
Features, on the other hand, are more complicated and migrating could be a bit difficult, but they're cleaner
I'd go with the //
approach because it's simpler
I don't quite understand the project structure. I want to make an isolated module that handles writing and reading from .leet.toml
. Where do I put the file to follow the existing structure?
Put it directly under lib.rs
It already has a weird config.rs
Yes that is what stores the command line arguments information. Actually, maybe we should try to merge the two. So information from .leet.toml
gets used as defaults for the config and the user can override it from the command line. What do you think?
Yes that is what stores the command line arguments information. Actually, maybe we should try to merge the two. So information from
.leet.toml
gets used as defaults for the config and the user can override it from the command line. What do you think?
Let me know what you think and we can go through it together because we'd need to change the values in the config to be able to differentiate between defaults and no value passed.
I just made a config_file.rs
beside it:
use std::fs;
use anyhow::Context;
use serde::{Deserialize, Serialize};
#[derive(Clone, Serialize, Deserialize, Debug, Default)]
pub(crate) struct ConfigFile {
pub active: Option<String>,
}
impl ConfigFile {
pub(crate) fn load() -> anyhow::Result<Self> {
let content = std::fs::read_to_string(".leet.toml").context("failed to read .leet.toml")?;
Ok(toml::from_str(&content)?)
}
pub(crate) fn save(&self) -> anyhow::Result<()> {
let content = toml::to_string(&self).context("failed to convert toml")?;
fs::write(".leet.toml", content).context("failed to write .leet.toml")?;
Ok(())
}
}
Seeing an empty struct in config.rs
felt weird. Wouldn't a module do the same?
Which one is the empty struct?
In src/tool/config.rs
:
pub(crate) struct Config {}
impl Config {
// assumed in the code using them URLs Must end with trailing "/"
pub(crate) const LEETCODE_PROBLEM_URL: &'static str = "https://leetcode.com/problems/";
pub(crate) const LEETCODE_GRAPH_QL: &'static str = "https://leetcode.com/graphql/";
}
In
src/tool/config.rs
:pub(crate) struct Config {} impl Config { // assumed in the code using them URLs Must end with trailing "/" pub(crate) const LEETCODE_PROBLEM_URL: &'static str = "https://leetcode.com/problems/"; pub(crate) const LEETCODE_GRAPH_QL: &'static str = "https://leetcode.com/graphql/"; }
ah I was looking in CLI... That was an early decision, we could consider revisiting it.
Currently testing (and in the future copying) a specific problem is a bit difficult, requiring a long command. What if we add a
.active
file to the project root, storing the currently active problem. We could then make each problem a feature, and with acargo leet test
subcommand test only the active problem, without having to rebuild anything else. This feature should be added on everycargo leet gen ...
.This isn't any breaking change, so anyone would still be able to use it normally, but we could possibly add a migration subcommand in the future, tacking a feature on every problem.
What do you think?