[GitHub] [guacamole-server] necouchman commented on a diff in pull request #383: GUACAMOLE-1622: Added margins to ssh sessions.

[GitBox <gi...@apache.org>]

necouchman commented on code in PR #383:
URL: https://github.com/apache/guacamole-server/pull/383#discussion_r894450130


##########
src/terminal/display.c:
##########
@@ -1020,7 +1035,11 @@ int guac_terminal_display_set_font(guac_terminal_display* display,
     if (new_width != display->width || new_height != display->height)
         guac_terminal_display_resize(display, new_width, new_height);
 
+
     return 0;
 
 }
 
+int get_margin_by_dpi(int dpi) {
+    return dpi * GUAC_TERMINAL_MARGINS / 25.4;

Review Comment:
   Maybe `25.4` should be defined as a constant somewhere, which would also help me understand why the value is chosen :-).



##########
src/terminal/terminal/terminal.h:
##########
@@ -279,6 +279,34 @@ guac_terminal_options* guac_terminal_options_create(
  */
 void guac_terminal_free(guac_terminal* term);
 
+/**
+ * Calculate the available space within the terminal 
+ * and store the results in the pointer arguments.
+ *
+ * @param terminal
+ *     The terminal provides character width and height for caluclations.
+ * 
+ * @param width
+ *     The width of the terminal, in pixels.
+ *
+ * @param height
+ *     The height of the terminal, in pixels.
+ * 
+ * @param aw
+ *     Set with the available width for text of the terminal, in pixels.
+ * 
+ * @param ah
+ *     Set with the available height for text of the terminal, in pixels.
+ * 
+ * @param columns
+ *     Set with the available width for text of the terminal, by column count.
+ * 
+ * @param rows
+ *     Set with the available height for text of the terminal, by row count.
+ */
+void get_available_dimensions(guac_terminal* term, int width, int height, 
+        int* aw, int* ah, int* columns, int* rows);
+

Review Comment:
   Looks like this is only used in `terminal.c` - is there a reason to make it available in the header, or should it be a `static` function limited to `terminal.c`?



##########
src/terminal/terminal/display.h:
##########
@@ -369,5 +380,16 @@ void guac_terminal_display_clear_select(guac_terminal_display* display);
 int guac_terminal_display_set_font(guac_terminal_display* display,
         const char* font_name, int font_size, int dpi);
 
+/**
+ * Calculate the size of margins around the terminal based on DPI.
+ *
+ * @param dpi
+ *     The resolution of the display in DPI.
+ *
+ * @return
+ *     Calculated size of margin in pixels
+ */
+int get_margin_by_dpi(int dpi);        
+

Review Comment:
   I don't see this function used anywhere outside of `display.c` - is there some reason it should be made available outside of the file, or perhaps it should be a `static inc` within `display.c`?



-- 
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: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org