mike-jumper commented on a change in pull request #373:
URL: https://github.com/apache/guacamole-server/pull/373#discussion_r814213710
##########
File path: src/terminal/color-scheme.c
##########
@@ -46,7 +46,7 @@
* @return
* Zero if the two strings are equal and non-zero otherwise.
*/
-static int guac_terminal_color_scheme_compare_token(const char* str_start,
+static int guac_terminal_get_color_scheme_compare_token(const char* str_start,
const char* str_end, const char* literal) {
Review comment:
While this does return an `int`, it's not a getter; it's a comparator. I
think the old naming was correct.
##########
File path: src/terminal/color-scheme.c
##########
@@ -70,7 +70,7 @@ static int guac_terminal_color_scheme_compare_token(const
char* str_start,
* Address of a pointer to the end of the string, after the last character.
* On return, the pointer is moved back to before any trailing spaces.
*/
-static void guac_terminal_color_scheme_strip_spaces(const char** str_start,
+static void guac_terminal_get_color_scheme_strip_spaces(const char** str_start,
Review comment:
Similar here: this isn't a getter, but a function which operates
directly on the given values. It's in the "color_scheme" namespace beneath the
"guac_terminal" namespace, thus has a `guac_terminal_color_scheme_` prefix, and
has the descriptive name `strip_spaces` after that describing the action
performed. I think that original naming was correct.
##########
File path: src/terminal/terminal/color-scheme.h
##########
@@ -17,13 +17,13 @@
* under the License.
*/
-#ifndef GUAC_TERMINAL_COLOR_SCHEME_H
-#define GUAC_TERMINAL_COLOR_SCHEME_H
+#ifndef guac_terminal_get_color_scheme_H
+#define guac_terminal_get_color_scheme_H
Review comment:
The convention for global macros in C is `UPPERCASE_WITH_UNDERSCORES`. I
think this may just be the accidental result of a case-insensitive
search-and-replace, but this should be `GUAC_TERMINAL_COLOR_SCHEME_H`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
