[GitHub] guacamole-server pull request #197: GUACAMOLE-287: Added Constants in place ...
Github user mike-jumper commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/197#discussion_r232502934 --- Diff: src/terminal/terminal/ansi_escape_codes.h --- @@ -0,0 +1,279 @@ +/* + * 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_ANSI_ESCAPE_CODES_H +#define _GUAC_TERMINAL_ANSI_ESCAPE_CODES_H + + +/** + * The maximum number of parameters the CSI can have is 16. + */ +#define GUAC_TERMINAL_MAX_ARGUMENTS 16 + +/** + * When the parameter '2' is preceded by the escape character + * with '[' and followed by the the character 'J' the entire + * display is cleared. Without the '2' the line is deleted + * only from the cursor to the end of the display. + */ +#define GUAC_TERMINAL_ERASE_DISPLAY 2 --- End diff -- Similar to the documentation for this macro, it may be worth considering embedding part of the context of this constant in the constant name. For example, if this value only has the defined meaning within a CSI sequence, then `GUAC_TERMINAL_CSI_ERASE_DISPLAY` might be a more sensible name. If that `J` you mention is yet another specific type of CSI sequence, then `GUAC_TERMINAL_CSI_WHATEVER_J_IS_ERASE_DISPLAY` would make yet more sense. ---
[GitHub] guacamole-server pull request #197: GUACAMOLE-287: Added Constants in place ...
Github user mike-jumper commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/197#discussion_r232502752 --- Diff: src/terminal/terminal/ansi_escape_codes.h --- @@ -0,0 +1,279 @@ +/* + * 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_ANSI_ESCAPE_CODES_H --- End diff -- As mentioned previously, macros should not start with leading underscores. You will see this in older code, yes, but it is incorrect. Please do not use leading underscores in new code. See: https://github.com/apache/guacamole-server/pull/197#discussion_r228667853 and https://github.com/apache/guacamole-server/pull/197#discussion_r229821057 ---
[GitHub] guacamole-server pull request #197: GUACAMOLE-287: Added Constants in place ...
Github user mike-jumper commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/197#discussion_r232503178 --- Diff: src/terminal/terminal/ansi_escape_codes.h --- @@ -0,0 +1,279 @@ +/* + * 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_ANSI_ESCAPE_CODES_H +#define _GUAC_TERMINAL_ANSI_ESCAPE_CODES_H + + +/** + * The maximum number of parameters the CSI can have is 16. + */ +#define GUAC_TERMINAL_MAX_ARGUMENTS 16 + +/** + * When the parameter '2' is preceded by the escape character + * with '[' and followed by the the character 'J' the entire --- End diff -- If the idea behind this change is to avoid the use of magic numbers, then the documentation shouldn't rely on magic numbers either. Ignoring that there are other ways to start a CSI sequence besides "`ESC` `[`", the `[` and the `J` have their own meanings in this context. There must be a better way to describe the use of this constant. ---
[GitHub] guacamole-server pull request #197: GUACAMOLE-287: Added Constants in place ...
Github user mike-jumper commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/197#discussion_r229823288 --- Diff: src/terminal/terminal/ansi_escape_codes.h --- @@ -0,0 +1,197 @@ +/* + * 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_ANSI_ESCAPE_CODES_H +#define __GUAC_TERMINAL_ANSI_ESCAPE_CODES_H + +#include "config.h" + +/** + * The maximum number of parameters the CSI can have is 16. + */ +#define GUAC_TERMINAL_MAX_ARGUMENTS 16 + +/** + * Erase entire screen and move cursor to upper left of ANSI.SYS. + */ +#define GUAC_TERMINAL_ERASE_DISPLAY 2 + +/** + * Erase entire screen and delete all lines saved in the + * scrollback buffer. + */ +#define GUAC_TERMINAL_ERASE_SCROLLBACK 3 + +/** + * The device status report(DSR) requests the terminal' operation + * stauts report. Always returns "CSI 0 n" (Terminal ready). --- End diff -- "stauts" ---
[GitHub] guacamole-server pull request #197: GUACAMOLE-287: Added Constants in place ...
Github user mike-jumper commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/197#discussion_r229821057 --- Diff: src/terminal/terminal/ansi_escape_codes.h --- @@ -0,0 +1,197 @@ +/* + * 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_ANSI_ESCAPE_CODES_H +#define __GUAC_TERMINAL_ANSI_ESCAPE_CODES_H --- End diff -- This `#define` needs to match the `#ifndef` above. See: https://en.wikipedia.org/wiki/Include_guard#Use_of_#include_guards Also, though you will see the `_` and `__` prefix used elsewhere in older code, that usage is incorrect as such prefixes are reserved for internal use by the language/compiler. Newer code will serve as a better example: https://github.com/apache/guacamole-server/blob/6f491946408224a231324663938df36c756b09d2/src/terminal/terminal/xparsecolor.h See also: https://en.wikipedia.org/wiki/Include_guard#Discussion ---
[GitHub] guacamole-server pull request #197: GUACAMOLE-287: Added Constants in place ...
Github user mike-jumper commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/197#discussion_r229824188 --- Diff: src/terminal/terminal/ansi_escape_codes.h --- @@ -0,0 +1,197 @@ +/* + * 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_ANSI_ESCAPE_CODES_H +#define __GUAC_TERMINAL_ANSI_ESCAPE_CODES_H + +#include "config.h" + +/** + * The maximum number of parameters the CSI can have is 16. + */ +#define GUAC_TERMINAL_MAX_ARGUMENTS 16 + +/** + * Erase entire screen and move cursor to upper left of ANSI.SYS. + */ +#define GUAC_TERMINAL_ERASE_DISPLAY 2 + +/** + * Erase entire screen and delete all lines saved in the + * scrollback buffer. + */ +#define GUAC_TERMINAL_ERASE_SCROLLBACK 3 + +/** + * The device status report(DSR) requests the terminal' operation + * stauts report. Always returns "CSI 0 n" (Terminal ready). + */ +#define GUAC_TERMINAL_DEVICE_STATUS_REPORT 5 + +/** + * The cursor position report (CPR) reports the current cursor + * position to the application. --- End diff -- It _requests_ that the terminal report the current position, yes? (Similar to your description for `GUAC_TERMINAL_DEVICE_STATUS_REPORT`) ---
[GitHub] guacamole-server pull request #197: GUACAMOLE-287: Added Constants in place ...
Github user mike-jumper commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/197#discussion_r229822193 --- Diff: src/terminal/terminal/ansi_escape_codes.h --- @@ -0,0 +1,197 @@ +/* + * 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_ANSI_ESCAPE_CODES_H +#define __GUAC_TERMINAL_ANSI_ESCAPE_CODES_H + +#include "config.h" + +/** + * The maximum number of parameters the CSI can have is 16. + */ +#define GUAC_TERMINAL_MAX_ARGUMENTS 16 + +/** + * Erase entire screen and move cursor to upper left of ANSI.SYS. --- End diff -- ANSI.SYS? ---
[GitHub] guacamole-server pull request #197: GUACAMOLE-287: Added Constants in place ...
Github user mike-jumper commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/197#discussion_r229823156 --- Diff: src/terminal/terminal/ansi_escape_codes.h --- @@ -0,0 +1,197 @@ +/* + * 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_ANSI_ESCAPE_CODES_H +#define __GUAC_TERMINAL_ANSI_ESCAPE_CODES_H + +#include "config.h" + +/** + * The maximum number of parameters the CSI can have is 16. --- End diff -- You don't need to include the value this macro expands to within the documentation for the macro itself. Part of the point of documentation and macros like this are to avoid the need to know such implementation-specific details. Something like "the maximum number of parameters the CSI can have" is probably almost perfect, though I recommend finding a better way to say "the CSI" that is in line with existing documentation and standards covering console codes. I've seen this referred to as "the CSI sequence" before, but never "the CSI". ---
[GitHub] guacamole-server pull request #197: GUACAMOLE-287: Added Constants in place ...
Github user mike-jumper commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/197#discussion_r229825489 --- Diff: src/terminal/terminal/ansi_escape_codes.h --- @@ -0,0 +1,197 @@ +/* + * 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_ANSI_ESCAPE_CODES_H +#define __GUAC_TERMINAL_ANSI_ESCAPE_CODES_H + +#include "config.h" + +/** + * The maximum number of parameters the CSI can have is 16. + */ +#define GUAC_TERMINAL_MAX_ARGUMENTS 16 + +/** + * Erase entire screen and move cursor to upper left of ANSI.SYS. + */ +#define GUAC_TERMINAL_ERASE_DISPLAY 2 + +/** + * Erase entire screen and delete all lines saved in the + * scrollback buffer. + */ +#define GUAC_TERMINAL_ERASE_SCROLLBACK 3 + +/** + * The device status report(DSR) requests the terminal' operation + * stauts report. Always returns "CSI 0 n" (Terminal ready). + */ +#define GUAC_TERMINAL_DEVICE_STATUS_REPORT 5 + +/** + * The cursor position report (CPR) reports the current cursor + * position to the application. + */ +#define GUAC_TERMINAL_CURSOR_POSITION_REPORT 6 + +/** + * With the parameter '3' all tabstops are deleted. --- End diff -- Please look through the documentation of this and the other new constants and ask: if someone were to see this constant within the code, find this header, and look up the documentation, would they then understand the constant? This description is definitely an improvement from the last iteration, but still assumes some hidden knowledge of the context of the constant's use. The parameter of what? Is there a sequence that this particular value must be part of to have the documented meaning? ---
[GitHub] guacamole-server pull request #197: GUACAMOLE-287: Added Constants in place ...
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/197#discussion_r228670354
--- Diff: src/terminal/terminal_handlers.c ---
@@ -1422,7 +1431,7 @@ int guac_terminal_ctrl_func(guac_terminal* term,
unsigned char c) {
switch (c) {
/* Alignment test (fill screen with E's) */
-case '8':
+case 'GUAC_TERMINAL_ALIGNMENT_TEST':
--- End diff --
This will not compile. Presumably you meant:
case GUAC_TERMINAL_ALIGNMENT_TEST:
and
#define GUAC_TERMINAL_ALIGNMENT_TEST '8'
---
[GitHub] guacamole-server pull request #197: GUACAMOLE-287: Added Constants in place ...
Github user mike-jumper commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/197#discussion_r228667920 --- Diff: src/terminal/ansi_escape_codes.h --- @@ -0,0 +1,152 @@ +/* + * 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_H +#define _GUAC_TERMINAL_H + #include "config.h" + /** --- End diff -- Please separate logical blocks with a blank line. ---
[GitHub] guacamole-server pull request #197: GUACAMOLE-287: Added Constants in place ...
Github user mike-jumper commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/197#discussion_r228667853 --- Diff: src/terminal/ansi_escape_codes.h --- @@ -0,0 +1,152 @@ +/* + * 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_H --- End diff -- 1. Why is this `#ifndef` (and the `#include` later) indented by one space? 2. The `_GUAC_TERMINAL_H` name is already used by `terminal.h`. Ignoring the leading underscore (which is technically incorrect and shouldn't have been done in `terminal.h`), convention is to convert the name of the header to uppercase with underscores and add the appropriate namespace. ---
[GitHub] guacamole-server pull request #197: GUACAMOLE-287: Added Constants in place ...
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/197#discussion_r228672433
--- Diff: src/terminal/terminal_handlers.c ---
@@ -469,7 +470,7 @@ static int guac_terminal_parse_xterm256_rgb(int argc,
const int* argv,
guac_terminal_color* color) {
/* RGB color palette entries require three arguments */
-if (argc < 3)
--- End diff --
I'm not sure this qualifies as a magic number. The various console codes
definitely do, but 3 here is not a mystical value; it's the number of arguments
required by specifically this sequence. It's worth documenting the format of
the sequence (presumably in the documentation for
`guac_terminal_parse_xterm256_rgb()` and/or the constants which define the
codes that begin that sequence), but I wouldn't think it needs its own constant.
---
[GitHub] guacamole-server pull request #197: GUACAMOLE-287: Added Constants in place ...
Github user mike-jumper commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/197#discussion_r228668050 --- Diff: src/terminal/ansi_escape_codes.h --- @@ -0,0 +1,152 @@ +/* + * 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_H +#define _GUAC_TERMINAL_H + #include "config.h" + /** + * Maximum Arguments --- End diff -- Maximum arguments of what? This is less magical than just "16", but I think there needs to be more context here for this constant to be truly magic free. ---
[GitHub] guacamole-server pull request #197: GUACAMOLE-287: Added Constants in place ...
Github user mike-jumper commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/197#discussion_r228669193 --- Diff: src/terminal/ansi_escape_codes.h --- @@ -0,0 +1,152 @@ +/* + * 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_H +#define _GUAC_TERMINAL_H + #include "config.h" + /** + * Maximum Arguments + */ +#define GUAC_TERMINAL_MAX_ARGUMENTS 16 + /** + * Erase Display + */ +#define GUAC_TERMINAL_ERASE_DISPLAY 2 + /** + * Erase Display including scrollbar + */ +#define GUAC_TERMINAL_ERASE_SCROLLBAR 3 + /** + * Device status report --- End diff -- Proper formatting of Doxygen comments is: /** * Something like this. */ It looks like things are weird here because the opening `/**` is indented by a stray space while the body of the comment is missing a period. This applies elsewhere as well. ---
[GitHub] guacamole-server pull request #197: GUACAMOLE-287: Added Constants in place ...
Github user mike-jumper commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/197#discussion_r228668762 --- Diff: src/terminal/ansi_escape_codes.h --- @@ -0,0 +1,152 @@ +/* + * 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_H +#define _GUAC_TERMINAL_H + #include "config.h" + /** + * Maximum Arguments + */ +#define GUAC_TERMINAL_MAX_ARGUMENTS 16 + /** + * Erase Display + */ +#define GUAC_TERMINAL_ERASE_DISPLAY 2 + /** + * Erase Display including scrollbar --- End diff -- The console code that this constant is intended to represent presumably doesn't deal with the scrollbar. Whether the terminal provides scrolling at all, implements it with a scrollbar, etc. is all outside the scope of what these codes normally deal with. What code is this for? ---
[GitHub] guacamole-server pull request #197: GUACAMOLE-287: Added Constants in place ...
Github user mike-jumper commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/197#discussion_r228669520 --- Diff: src/terminal/ansi_escape_codes.h --- @@ -0,0 +1,152 @@ +/* + * 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_H +#define _GUAC_TERMINAL_H + #include "config.h" + /** + * Maximum Arguments + */ +#define GUAC_TERMINAL_MAX_ARGUMENTS 16 + /** + * Erase Display + */ +#define GUAC_TERMINAL_ERASE_DISPLAY 2 + /** + * Erase Display including scrollbar + */ +#define GUAC_TERMINAL_ERASE_SCROLLBAR 3 + /** + * Device status report + */ +#define GUAC_TERMINAL_DEVICE_STATUS_REPORT 5 + /** + * Cursor position report + */ +#define GUAC_TERMINAL_CURSOR_POSITION_REPORT 6 + /** + * Clear tab stop at current position + */ +#define GUAC_TERMINAL_CLEAR_TAB_STOPS 3 + /** + * Erase from start of line to cursor + */ +#define GUAC_TERMINAL_ERASE_WHOLE_LINE 2 + /** + * Set scrolling region + * parameters are top and bottom row + */ +#define GUAC_TERMINAL_SET_SCROLLING_REGION 2 + /** + * Set window title + */ +#define GUAC_TERMINAL_SET_WINDOW_TITLE 2 + /** + * Set ANSI Color + */ +#define GUAC_TERMINAL_SET_COLOR 4 + /** + * Faint(Decreased Intensity) + */ +#define GUAC_TERMINAL_FAINT 2 + /** + * Underline + */ +#define GUAC_TERMINAL_UNDERLINE_ON 4 + /** + * Reverse Video + */ +#define GUAC_TERMINAL_REVERSE_VIDEO 7 + /** + * Doubly Underline or Bold off + */ +#define GUAC_TERMINAL_DOUBLY_UNDERLINED 21 + /** + * Normal color or intensity + */ +#define GUAC_TERMINAL_NORMAL_INTENSITY 22 + /** + * Underline off + */ +#define GUAC_TERMINAL_UNDERLINE_OFF 24 + /** + * Inverse off + */ +#define GUAC_TERMINAL_REVERSE_VIDEO_OFF 27 + /** + * Set Black Foreground + */ +#define GUAC_TERMINAL_BLACK_FOREGROUND 30 + /** + * Set White Foreground + */ +#define GUAC_TERMINAL_WHITE_FOREGROUND 37 + /** + * Set Underscore on, default Foreground color + */ +#define GUAC_TERMINAL_DEF_FOREGROUND_UNDERSCORE_ON 38 + /** + * Set Underscore off, default Foreground color + */ +#define GUAC_TERMINAL_DEF_FOREGROUND_UNDERSCORE_OFF 39 + /** + * Set Bright Foreground Color - lower bound + */ +#define GUAC_TERMINAL_BRIGHT_FOREGROUND_LOW 90 + /** + * Set Bright Foreground Color - upper bound + */ +#define GUAC_TERMINAL_BRIGHT_FOREGROUND_HIGH 97 + /** + * Set Black Background + */ +#define GUAC_TERMINAL_BLACK_BACKGROUND 40 + /** + * Set White Background + */ +#define GUAC_TERMINAL_WHITE_BACKGROUND 47 + /** + * Set Background color + * Next arguments are 5;n or 2;r;g;b + */ +#define GUAC_TERMINAL_SET_BACKGROUND 48 + /** + * Set Default Black Background color + */ +#define GUAC_TERMINAL_DEFAULT_BACKGROUND 49 + /** + * Set Bright Background Color - lower bound + */ +#define GUAC_TERMINAL_BRIGHT_BACKGROUND_LOW 100 + /** + * Set Bright Background Color - upper bound + */ +#define GUAC_TERMINAL_BRIGHT_BACKGROUND_HIGH 107 + /** + * RGB Colors --- End diff -- Please clarify. Given the constant `GUAC_TERMINAL_MAX_COLORS`, what does the description "RGB Colors" mean? ---
[GitHub] guacamole-server pull request #197: GUACAMOLE-287: Added Constants in place ...
GitHub user m-khan-glyptodon opened a pull request: https://github.com/apache/guacamole-server/pull/197 GUACAMOLE-287: Added Constants in place of magic numbers The constants are defined in the "ansi_escape_codes.h" file You can merge this pull request into a Git repository by running: $ git pull https://github.com/m-khan-glyptodon/guacamole-server magic-numbers Alternatively you can review and apply these changes as the patch at: https://github.com/apache/guacamole-server/pull/197.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #197 commit 209054ea75a8aa534817b19517fd9570bb23bb87 Author: m-khan-glyptodon Date: 2018-10-26T20:09:53Z GUACAMOLE-287: Added constants for magic numbers ---
