Skip to content

Conversation

@dfayd0
Copy link
Member

@dfayd0 dfayd0 commented Jun 7, 2025

This pull request introduces several significant changes to the leetcode-cli project, focusing on adding a new feature for parsing problem README files, improving API handling, and enhancing error handling and flexibility. The changes also include updates to dependencies and tests.

New Feature: README Parsing

  • Added a new module readme_parser to parse problem README files, extracting example counts, inputs, and outputs using regular expressions. (src/readme_parser.rs, src/readme_parser.rsR1-R61)
  • Included test cases for the readme_parser module to validate its functionality with sample README files. (tests/readme_parser_tests.rs, tests/readme_parser_tests.rsR1-R116)
  • Added sample README files for testing purposes. (tests/data/221.md, [1]; tests/data/3467.md, [2]; tests/data/392.md, [3]

API and Command Enhancements

  • Updated the LeetcodeApiRunner::new method to take a reference to RuntimeConfigSetup instead of consuming it, improving memory efficiency. (src/leetcode_api_runner.rs, src/leetcode_api_runner.rsL26-R29)
  • Modified the main function to reflect the changes in LeetcodeApiRunner and to handle the default programming language when none is provided. (src/main.rs, [1] [2]
  • Changed the language field in the Commands enum to be an Option<String> to allow for optional language input. (src/cli.rs, src/cli.rsL23-R23)

Error Handling Improvements

  • Updated the parse_programming_language function to return a Result instead of panicking on unsupported languages, improving error handling. (src/utils.rs, src/utils.rsL37-R65)

Dependency and Configuration Updates

  • Added the regex crate as a new dependency to support the readme_parser module. (Cargo.toml, Cargo.tomlR38)
  • Standardized the binary name in Cargo.toml from leetcode_cli to leetcode-cli. (Cargo.toml, Cargo.tomlL17-R17)

These changes collectively enhance the functionality, robustness, and maintainability of the leetcode-cli project.

@dfayd0 dfayd0 requested a review from guilhem-sante June 7, 2025 10:03
@dfayd0 dfayd0 self-assigned this Jun 7, 2025
@dfayd0 dfayd0 linked an issue Jun 7, 2025 that may be closed by this pull request
@guilhem-sante guilhem-sante added the enhancement New feature or request label Jun 7, 2025
Copy link
Member

@guilhem-sante guilhem-sante left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tests are failing 💀

@dfayd0
Copy link
Member Author

dfayd0 commented Jun 7, 2025

dans tous les cas je pense qu'on test pas encore assez fort pour pouvoir la mettre sur main

@guilhem-sante
Copy link
Member

guilhem-sante commented Jun 8, 2025

T'inquiètes te freines pas la dessus. Avance sur ton projet sinon tu vas te saouler.

Btw: hors du scope de la PR mais tu mets la configuration en ~/leetcode ça serait pas mieux ~/.leetcode?

@guilhem-sante guilhem-sante self-requested a review June 8, 2025 13:02
@dfayd0
Copy link
Member Author

dfayd0 commented Jun 9, 2025

Btw: hors du scope de la PR mais tu mets la configuration en ~/leetcode ça serait pas mieux ~/.leetcode?

La config est dans ~/.config/leetcode-cli/config.toml si je me trompe pas. le ~/leetcode c'est le folder (repo?) du user, qui contient les problèmes.

@guilhem-sante
Copy link
Member

Btw: hors du scope de la PR mais tu mets la configuration en ~/leetcode ça serait pas mieux ~/.leetcode?

La config est dans ~/.config/leetcode-cli/config.toml si je me trompe pas. le ~/leetcode c'est le folder (repo?) du user, qui contient les problèmes.

On veut pas créer le dossier du probleme en local comme un git clone ?

@dfayd0
Copy link
Member Author

dfayd0 commented Jun 15, 2025

Btw: hors du scope de la PR mais tu mets la configuration en ~/leetcode ça serait pas mieux ~/.leetcode?

La config est dans ~/.config/leetcode-cli/config.toml si je me trompe pas. le ~/leetcode c'est le folder (repo?) du user, qui contient les problèmes.

On veut pas créer le dossier du probleme en local comme un git clone ?

J'avais pas vu my bad!
Je voyais plutôt ca avec un repo central, et dedans chaque exo, je sais pas si ca fait sens pour toi.

@dfayd0
Copy link
Member Author

dfayd0 commented Jun 15, 2025

Je te re review sur ca!
relis bien et hésite pas a comment, mais normalement je pense qu'on est pas trop mal sur la

mais les problèmes sont clairement sur #10 ;')

raw: readme.to_string(),
}
}
pub fn parse(&self) -> Result<ProblemTestData, Box<dyn std::error::Error>> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

J'ai enfin lu le guide sur l'error handling en Rust dont je te parlais et de ce que je lisais il faudrait éviter les Box<dyn Error> comme ça.
Ca rend leur management programmatique difficile (comme un conditionel sur le type d'erreur rendu), c'est surtout utile quand tu sais que tu veux output une erreur qui n'est que destiné qu'à être logger dans le terminal (donc plutôt les erreurs qui sont exposées au end-user).

Dans le cas d'une méthode que tu utilises aux seins de ton programme comme ça et qui pourrait nécessiter d'être intéragit avec c'est mieux de déclarer des erreurs de manières static. C'est généralement assez long mais tu peux vraiment vivifier la chose avec thiserror.

Je vois si je peux pas te faire ça à la suite dans un commit de la PR.

…error in LeetcodeReadmeParser signature functions
Copy link
Member

@guilhem-sante guilhem-sante left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

J'approuve la PR du coup rien à dire sur le reste. Je te mets en review de la PR pour que tu checks le changement que je t'ai suggérer du coup.

Aussi, après mon changement j'ai voulu checker ta suite de test mais 1 test échouait et en le regardant j'ai supposé que c'était dû à mon absence de token leetcode. Donc je pense que c'est bon mais revérfie si tu peux pour être sûr.

Comment on lines 14 to 36
#[derive(thiserror::Error, Debug, Clone, Copy, PartialEq, Eq)]
pub enum LeetcodeReadmeParserError {
#[error("can't parse empty readme")]
EmptyReadme,
}

impl LeetcodeReadmeParser {
pub fn new(readme: &str) -> Self {
LeetcodeReadmeParser {
raw: readme.to_string(),
}
}

pub fn parse(&self) -> Result<ProblemTestData, LeetcodeReadmeParserError> {
if self.raw.is_empty() {
return Err(LeetcodeReadmeParserError::EmptyReadme);
}
Ok(ProblemTestData {
example_count: self.count_examples(),
inputs: self.extract_inputs(),
outputs: self.extract_outputs(),
})
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Voilà, ça donnerait un truc comme ça du coup. Avec une erreur définie statiquement.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Archi chaud!
Merci mec c'est super cool!
En revanche t'as commit 10 autres fichiers pour du styling, c'est accidentel?

@guilhem-sante
Copy link
Member

J'approuve la PR du coup rien à dire sur le reste. Je te mets en review de la PR pour que tu checks le changement que je t'ai suggérer du coup.

Aussi, après mon changement j'ai voulu checker ta suite de test mais 1 test échouait et en le regardant j'ai supposé que c'était dû à mon absence de token leetcode. Donc je pense que c'est bon mais revérfie si tu peux pour être sûr.

Bon je peux pas te mettre en review de ta propre PR mais je te laisse me donner ton avis sur la question des erreurs en tout cas.

@dfayd0
Copy link
Member Author

dfayd0 commented Jun 18, 2025

Bon je peux pas te mettre en review de ta propre PR mais je te laisse me donner ton avis sur la question des erreurs en tout cas.

Bah ecoute je viens de review, et c'est super quali merci beaucoup!
En revanche si tu peux retirer les fichiers modifié je pense par ton crtl+S c'est top

@dfayd0 dfayd0 merged commit 8892e15 into main Jun 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

parse readme to collect test values

3 participants