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

fix tcl configuration parse error #1768

Merged
merged 2 commits into from
Jul 20, 2023

Conversation

A2ureStone
Copy link
Contributor

@@ -210,7 +207,7 @@ proc start_server {options {code undefined}} {
} elseif {$directive == "dump_prefix"} {
puts $fp "$directive :"
} else {
puts -nonewline $fp "$directive "
puts -nonewline $fp "$directive : "
puts $fp [dict get $config $directive]
}
}
Copy link

Choose a reason for hiding this comment

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

Overall, the code patch seems to involve changes in handling and writing configuration directives. Here are some observations and suggestions:

  1. Line 178: set arguments [lrange $elements 2 end] appears to be an incorrect change. It should remain as set arguments [lrange $elements 1 end] to capture the correct arguments.

  2. Lines 202-206: The conditions for writing directive values could be simplified. Instead of explicitly checking for "port," "requirepass," and "userpass" directives, you can use a generic approach to handle all directives.

    Here's an example of how you can simplify those lines:

    puts -nonewline $fp "$directive : "
    puts $fp [dict get $config $directive]
    
  3. Line 210: In the case of the "dump_prefix" directive, there is an extra space after the colon ":". To align with the existing code, use puts $fp "$directive:" instead.

  4. Consider adding error handling for file operations like opening and closing files. You can use catch or try to handle any exceptions that might occur during file operations.

  5. It would be helpful to include comments in the code to explain the purpose of certain sections or highlight any complex logic.

Remember to thoroughly test the code after making these modifications, especially the changes related to reading and writing configuration directives, to ensure they work as expected.

@AlexStocks AlexStocks merged commit 6ac7b7f into OpenAtomFoundation:unstable Jul 20, 2023
8 checks passed
bigdaronlee163 pushed a commit to bigdaronlee163/pika that referenced this pull request Jun 8, 2024
cheniujh pushed a commit to cheniujh/pika that referenced this pull request Sep 24, 2024
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.

2 participants