Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace Multiline Strings with indoc!() for Better Readability and Maintainability #40

Merged
merged 2 commits into from
Mar 14, 2025

Conversation

mentatbot[bot]
Copy link
Contributor

@mentatbot mentatbot bot commented Mar 14, 2025

This commit addresses issue #39 by replacing multiline string literals with the indoc!() macro. This macro automatically removes common leading whitespace from multiline strings, ensuring that the string content remains correctly formatted regardless of its indentation level in the code.

Changes include:

  • Added indoc = "1.0" to dependencies in Cargo.toml
  • Updated imports to include the indoc macro in all relevant files
  • Replaced raw string literals (r#"..."#) with indoc! in:
    • benches/parsing_benchmarks.rs (language samples)
    • src/db.rs (SQL queries)
    • src/prompt/domain_extraction.rs (prompt templates)
    • src/prompt/llm_integration.rs (mock responses)
    • src/parser/language_support/*.rs (test data)
    • src/mcp_server/router.rs (instructions)
    • src/query/nl_translator.rs (translation prompts)
    • src/query/parser.rs (added import only)

These changes improve the maintainability of the codebase by ensuring that string indentation is automatically handled, preventing issues when code is refactored or moved.

Closes #39


🤖 See my steps and cost here

  • Wake on any new activity.

…intainability

This commit addresses issue # 39 by replacing multiline string literals with the indoc!() macro. This macro automatically removes common leading whitespace from multiline strings, ensuring that the string content remains correctly formatted regardless of its indentation level in the code.

Changes include:
- Added indoc = "1.0" to dependencies in Cargo.toml
- Updated imports to include the indoc macro in all relevant files
- Replaced raw string literals (r#"..."#) with indoc! in:
  - benches/parsing_benchmarks.rs (language samples)
  - src/db.rs (SQL queries)
  - src/prompt/domain_extraction.rs (prompt templates)
  - src/prompt/llm_integration.rs (mock responses)
  - src/parser/language_support/*.rs (test data)
  - src/mcp_server/router.rs (instructions)
  - src/query/nl_translator.rs (translation prompts)
  - src/query/parser.rs (added import only)

These changes improve the maintainability of the codebase by ensuring that string indentation is automatically handled, preventing issues when code is refactored or moved.

Closes # 39
@mentatbot mentatbot bot requested a review from Nayshins March 14, 2025 17:10
Copy link
Contributor Author

mentatbot bot commented Mar 14, 2025

I've completed the implementation of issue #39, replacing multiline strings with the indoc!() macro throughout the codebase.

Changes Made:

  1. Added indoc = "1.0" to dependencies in Cargo.toml
  2. Added the appropriate imports (use indoc::indoc;) to all files using multiline strings
  3. Replaced raw string literals (r#"..."#) with indoc! {r#"..."#} in:
    • benches/parsing_benchmarks.rs (language samples)
    • src/db.rs (SQL queries)
    • src/prompt/domain_extraction.rs (prompt templates)
    • src/prompt/llm_integration.rs (mock responses)
    • src/parser/language_support/*.rs (test data)
    • src/mcp_server/router.rs (instructions)
    • src/query/nl_translator.rs (translation prompts)

Benefits:

  • Improved readability with consistent indentation
  • Better maintainability when code is refactored or moved
  • Automatic handling of indentation without manual adjustments
  • Consistent formatting of multiline strings throughout the codebase

The changes are purely stylistic and shouldn't affect any functionality. The indoc!() macro simply removes common leading whitespace, ensuring the string content remains correctly formatted regardless of its indentation level in the code.

This commit addresses the cargo fmt warning in the CI build by fixing the formatting of the indoc!() macro with to_string() call. The to_string() method call has been moved to a new line after the indoc! block to comply with Rust's formatting guidelines.

This is a minor formatting fix to address CI failures in PR # 40.
Copy link
Contributor Author

mentatbot bot commented Mar 14, 2025

I've pushed a small fix to address the CI failure. The issue was with the formatting of the indoc! macro in src/prompt/llm_integration.rs. According to Rust's formatting standards, when chaining methods after a multiline macro like indoc!, the method calls should start on a new line.

Fixed by changing:

return Ok(indoc! {r#"[
    // content
]"#}.to_string());

To:

return Ok(indoc! {r#"[
    // content
]"#}
.to_string());

The CI should now pass with this formatting fix.

@Nayshins
Copy link
Owner

What if we changed to use regular strings instead of raw string literals?

Copy link
Contributor Author

mentatbot bot commented Mar 14, 2025

@Nayshins That's a good question! We could certainly consider using regular strings ("...") instead of raw string literals (r#"..."#) in some cases.

The tradeoff to consider:

Raw string literals advantages:

  • Don't require escaping of quotes, backslashes, and other special characters
  • Very useful for SQL queries, code examples, and other text that contains quotes

Regular strings advantages:

  • Cleaner syntax without the r# prefix/suffix
  • More familiar to developers from other languages

In many of the cases we're handling (like SQL queries, code samples, and JSON), the content contains quotes and special characters that would require escaping in regular strings, making the raw string literals more maintainable.

For example, in SQL queries like:

SELECT * FROM users WHERE name = 'something'

The single quotes would need to be escaped in regular strings.

Or in code examples where we have things like string literals within the example:

let x = "hello world";

However, if you'd prefer, I can convert some or all of these to regular strings instead. Would you like me to make this change for all strings, or perhaps just for certain types of content that don't contain many special characters?

@Nayshins
Copy link
Owner

That makes total sense! Lets stick with the raw string literals then!

Copy link
Contributor Author

mentatbot bot commented Mar 14, 2025

Great! I'll keep the raw string literals combined with indoc!() as is. It does seem like the best approach for the code samples, SQL, and other content with quotes and special characters in this codebase.

The PR is now passing CI and all the requested changes have been implemented. Let me know if you'd like any other adjustments!

@Nayshins
Copy link
Owner

LGTM!

@Nayshins Nayshins merged commit 410e5be into master Mar 14, 2025
1 check passed
Copy link
Contributor Author

mentatbot bot commented Mar 14, 2025

Thank you for the review! All changes are complete and CI is passing. The PR has successfully:

  1. Added the indoc crate dependency
  2. Implemented the indoc!() macro for all multiline string literals
  3. Fixed formatting to ensure proper style compliance

This improvement will make the codebase more maintainable when refactoring code with multiline strings.

Copy link
Contributor Author

mentatbot bot commented Mar 14, 2025

Great! Thanks for merging the PR. I'm glad I could help improve the codebase with this enhancement to string handling.

@Nayshins Nayshins deleted the mentat-39-1 branch March 14, 2025 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace Multiline Strings with indoc!() for Better Readability and Maintainability
1 participant