mike-jumper commented on a change in pull request #373:
URL: https://github.com/apache/guacamole-server/pull/373#discussion_r812481582
##########
File path: src/protocols/kubernetes/argv.c
##########
@@ -53,8 +53,9 @@ int guac_kubernetes_argv_callback(guac_user* user, const
char* mimetype,
}
/* Update Kubernetes terminal size */
- guac_kubernetes_resize(client, terminal->term_height,
- terminal->term_width);
+ guac_kubernetes_resize(client,
+ guac_terminal_term_height(terminal),
+ guac_terminal_term_width(terminal));
Review comment:
Thoughts on avoiding the redundancy of `guac_terminal_term_height()`,
etc. while avoiding pixels vs. characters confusion? `guac_terminal_rows()`?
`guac_terminal_get_rows()`?
##########
File path: src/terminal/terminal/terminal.h
##########
@@ -591,15 +172,6 @@ typedef struct guac_terminal_options {
*/
guac_client* client;
Review comment:
I didn't notice this in the previous iteration, but I don't think this
should be an "option" either. It's the sort of thing that would be provided at
terminal creation and then never change.
##########
File path: src/terminal/terminal/terminal_priv.h
##########
@@ -0,0 +1,446 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+
+#ifndef _GUAC_TERMINAL_PRIV_H
+#define _GUAC_TERMINAL_PRIV_H
Review comment:
You'll see this in a lot of older code, but underscore prefixes are
apparently reserved for internal use by the C compiler/preprocessor when it
comes to global identifiers like these. It's not uncommon to do this anyway,
but knowing this now, we should avoid it. This should instead by
`GUAC_TERMINAL_PRIV_H`.
The underscore prefix is apparently technically only allowed for things like
structure members, and then only if it's not followed by a capital letter or a
second underscore.
##########
File path: src/terminal/terminal/terminal.h
##########
@@ -915,6 +509,75 @@ void guac_terminal_scroll_handler(guac_terminal_scrollbar*
scrollbar, int value)
void guac_terminal_dup(guac_terminal* term, guac_user* user,
guac_socket* socket);
+/**
+ * Returns the number of rows within the buffer of the given terminal which are
+ * not currently displayed on screen. Adjustments to the desired scrollback
+ * size are taken into account, and rows which are within the buffer but
+ * unavailable due to being outside the desired scrollback range are ignored.
+ *
+ * @param term
+ * The terminal whose off-screen row count should be determined.
+ *
+ * @return
+ * The number of rows within the buffer which are not currently displayed
+ * on screen.
+ */
+int guac_terminal_available_scroll(guac_terminal* term);
+
+/**
+ * Returns the height of the given terminal, in characters.
+ *
+ * @param term
+ * The terminal whose height should be determined.
+ *
+ * @return
+ * The height of the terminal, in characters.
+ */
+int guac_terminal_term_height(guac_terminal* term);
+
+/**
+ * Returns the width of the given terminal, in characters.
+ *
+ * @param term
+ * The terminal whose width should be determined.
+ *
+ * @return
+ * The width of the terminal, in characters.
+ */
+int guac_terminal_term_width(guac_terminal* term);
+
+/**
+ * Clears the clipboard contents for a given terminal, and assigns a new
+ * mimetype for future data.
+ *
+ * @param terminal The terminal whose clipboard is being reset.
+ * @param mimetype The mimetype of future data.
Review comment:
Please migrate to the newer style we use for `@param`, `@return`, etc.
For example:
https://github.com/apache/guacamole-server/blob/6dd33a8d90907b93a29bc482e888e4a972a0be4d/src/terminal/terminal/terminal.h#L874-L878
--
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]
