Skip to content
This repository has been archived by the owner on Nov 11, 2018. It is now read-only.

IND/RI/NEL control sequence implementations #350

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

IND/RI/NEL control sequence implementations #350

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4741f7f
defined new sequences, marked implemented sequences, implemented REVE…
cepko33 Aug 24, 2014
3d74757
added NL and IND control sequences, also failures
cepko33 Aug 31, 2014
85fa067
removed debug flag
cepko33 Aug 31, 2014
ab5a9bd
man scrolls up and down
cepko33 Aug 31, 2014
64d5dd4
Merge branch 'next-line' into xterm-seq
cepko33 Aug 31, 2014
5921139
commented debug message
cepko33 Sep 3, 2014
796b054
removed unnecessary scroll
cepko33 Sep 7, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion src/TerminalOutput.vala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ public class TerminalOutput : Gee.ArrayList<OutputLine> {
public void parse_stream_element(TerminalStream.StreamElement stream_element) {
switch (stream_element.stream_element_type) {
case TerminalStream.StreamElement.StreamElementType.TEXT:
//message(_("Text sequence received: '%s'"), stream_element.text);
message(_("Text sequence received: '%s'"), stream_element.text);
Copy link
Owner

Choose a reason for hiding this comment

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

This should probably be commented again as it slows some large terminal programs to a crawl.

Copy link
Author

Choose a reason for hiding this comment

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

commented


// Print only text that has not been printed yet
string text_left = stream_element.text.substring(
Expand Down Expand Up @@ -124,6 +124,8 @@ public class TerminalOutput : Gee.ArrayList<OutputLine> {
// This code causes a line feed or a new line operation
// TODO: Does LF always imply CR?
move_cursor(cursor_position.line + 1, 0);
terminal.terminal_view.terminal_output_view.add_line_views();
terminal.terminal_view.terminal_output_view.scroll_to_position();
Copy link
Owner

Choose a reason for hiding this comment

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

These two lines should not be necessary for the code to work (follow the move_cursor -> line_added -> on_output_line_added path). Did you encounter problems without them?

Copy link
Author

Choose a reason for hiding this comment

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

When these lines are omitted, scrolling down, up, and then back down in man has very strange behavior. The terminal will only catch up to the downward scrolling once it hits new text.

break;

case TerminalStream.StreamElement.ControlSequenceType.HORIZONTAL_TAB:
Expand Down Expand Up @@ -223,6 +225,24 @@ public class TerminalOutput : Gee.ArrayList<OutputLine> {
break;

case TerminalStream.StreamElement.ControlSequenceType.CURSOR_FORWARD:
case TerminalStream.StreamElement.ControlSequenceType.REVERSE_INDEX:
screen_offset -= 1;
move_cursor(cursor_position.line - stream_element.get_numeric_parameter(0,1), cursor_position.column);
terminal.terminal_view.terminal_output_view.add_line_views();
Copy link
Owner

Choose a reason for hiding this comment

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

Same here. This should be taken care of automatically. If it isn't, there is a bug.

Copy link
Author

Choose a reason for hiding this comment

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

When these lines are ommitted, the terminal cannot follow the upward scroll in man and the status line moves upward in the terminal as opposed to the man text scrolling downward

Copy link
Owner

Choose a reason for hiding this comment

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

While I cannot reproduce that problem, I occasionally see similar issues, which seem to be caused by https://github.com/p-e-w/finalterm/blob/master/src/Terminal.vala#L109, a piece of code that sometimes does not work the way it should according to the GLib API docs.

In any case, I'm afraid that calling add_line_views, or any other TerminalView method here is not what we want at all because it breaks the otherwise fairly clean MVC separation. We should fix the scrolling problem separately but this is not a viable workaround.

I'll try to dig a bit deeper and maybe fix it entirely so I can merge a clean version of this PR.

Copy link
Author

Choose a reason for hiding this comment

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

I'll take a look at the code you mentioned, I definitely would like to get more familiar with the code and the mvc pattern you've implemented

Copy link
Owner

Choose a reason for hiding this comment

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

Great! If you need any pointers, just drop me a line.

Copy link
Owner

Choose a reason for hiding this comment

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

Did you have the chance to revisit this? Meanwhile, I've made some changes to the way scrolling works, maybe that improved the situation already. Even if that should not be the case and those lines are indeed necessary, I'd probably still like to merge your PR since it fixes many problems with terminal programs.

Copy link
Author

Choose a reason for hiding this comment

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

I've taken on a job which makes FOSS difficult. If you think the existing code is fixing some problems, please feel free to merge!

Copy link
Owner

Choose a reason for hiding this comment

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

I understand. Thank you anyway for your contribution and good luck in your new job!

break;

case TerminalStream.StreamElement.ControlSequenceType.NEXT_LINE:
screen_offset += 1;
move_cursor(cursor_position.line + stream_element.get_numeric_parameter(0,1), 0);
terminal.terminal_view.terminal_output_view.add_line_views();
break;

case TerminalStream.StreamElement.ControlSequenceType.INDEX:
screen_offset += 1;
move_cursor(cursor_position.line + stream_element.get_numeric_parameter(0,1), cursor_position.column);
terminal.terminal_view.terminal_output_view.add_line_views();
break;

case TerminalStream.StreamElement.ControlSequenceType.CHARACTER_POSITION_RELATIVE:
// The CUF sequence moves the active position to the right.
// The distance moved is determined by the parameter (default: 1)
Expand Down
86 changes: 47 additions & 39 deletions src/TerminalStream.vala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,18 +208,18 @@ public class TerminalStream : Object {
// Naming convention follows xterm specification found at
// http://invisible-island.net/xterm/ctlseqs/ctlseqs.html
public enum ControlSequenceType {
UNKNOWN,
UNKNOWN, //implemented

BELL,
BACKSPACE,
CARRIAGE_RETURN,
BELL, //implemented
BACKSPACE, //implemented
CARRIAGE_RETURN, //implemented
RETURN_TERMINAL_STATUS,
FORM_FEED,
LINE_FEED,
FORM_FEED, //implemented
LINE_FEED, //implemented
SHIFT_IN,
SHIFT_OUT,
HORIZONTAL_TAB,
VERTICAL_TAB,
HORIZONTAL_TAB, //implemented
VERTICAL_TAB, //implemented

SEVEN_BIT_CONTROLS,
EIGHT_BIT_CONTROLS,
Expand All @@ -233,7 +233,7 @@ public class TerminalStream : Object {
DEC_SCREEN_ALIGNMENT_TEST,
SELECT_DEFAULT_CHARACTER_SET,
SELECT_UTF8_CHARACTER_SET,
DESIGNATE_G0_CHARACTER_SET_VT100,
DESIGNATE_G0_CHARACTER_SET_VT100, //implemented
DESIGNATE_G1_CHARACTER_SET_VT100,
DESIGNATE_G2_CHARACTER_SET_VT220,
DESIGNATE_G3_CHARACTER_SET_VT220,
Expand All @@ -244,8 +244,8 @@ public class TerminalStream : Object {
SAVE_CURSOR,
RESTORE_CURSOR,
FORWARD_INDEX,
APPLICATION_KEYPAD,
NORMAL_KEYPAD,
APPLICATION_KEYPAD, //implemented
NORMAL_KEYPAD, //implemented
CURSOR_TO_LOWER_LEFT_CORNER_OF_SCREEN,
FULL_RESET,
MEMORY_LOCK,
Expand All @@ -256,58 +256,63 @@ public class TerminalStream : Object {
INVOKE_G2_CHARACTER_SET_AS_GR,
INVOKE_G1_CHARACTER_SET_AS_GR,

INDEX,
NEXT_LINE,
TAB_SET,
REVERSE_INDEX,

USER_DEFINED_KEYS,
REQUEST_STATUS_STRING,
SET_TERMCAP_DATA,
REQUEST_TERMCAP_STRING,

INSERT_CHARACTERS,
CURSOR_UP,
CURSOR_DOWN,
CURSOR_FORWARD,
CURSOR_BACKWARD,
CURSOR_UP, //implemented
CURSOR_DOWN, //implemented
CURSOR_FORWARD, //implemented
CURSOR_BACKWARD, //implemented
CURSOR_NEXT_LINE,
CURSOR_PRECEDING_LINE,
CURSOR_CHARACTER_ABSOLUTE,
CURSOR_POSITION,
CURSOR_CHARACTER_ABSOLUTE, //implemented
CURSOR_POSITION, //implemented
CURSOR_FORWARD_TABULATION,
ERASE_IN_DISPLAY_ED,
ERASE_IN_DISPLAY_ED, //implemented
ERASE_IN_DISPLAY_DECSED,
ERASE_IN_LINE_EL,
ERASE_IN_LINE_EL, //implemented
ERASE_IN_LINE_DECSEL,
INSERT_LINES,
DELETE_LINES,
DELETE_CHARACTERS,
DELETE_CHARACTERS, //implemented
SCROLL_UP_LINES,
SCROLL_DOWN_LINES,
INITIATE_HIGHLIGHT_MOUSE_TRACKING,
RESET_TITLE_MODES_FEATURES,
ERASE_CHARACTERS,
ERASE_CHARACTERS, //implemented
CURSOR_BACKWARD_TABULATION,
CHARACTER_POSITION_ABSOLUTE,
CHARACTER_POSITION_ABSOLUTE, //implemented
CHARACTER_POSITION_RELATIVE,
REPEAT_PRECEDING_GRAPHIC_CHARACTER,
SEND_DEVICE_ATTRIBUTES_PRIMARY,
SEND_DEVICE_ATTRIBUTES_SECONDARY,
LINE_POSITION_ABSOLUTE,
LINE_POSITION_RELATIVE,
HORIZONTAL_AND_VERTICAL_POSITION,
LINE_POSITION_ABSOLUTE, //implemented
LINE_POSITION_RELATIVE, //implemented
HORIZONTAL_AND_VERTICAL_POSITION, //implemented
TAB_CLEAR,
SET_MODE,
SET_MODE, //implemented
DEC_PRIVATE_MODE_SET,
MEDIA_COPY,
MEDIA_COPY_DEC,
RESET_MODE,
DEC_PRIVATE_MODE_RESET,
CHARACTER_ATTRIBUTES,
RESET_MODE, //implemented
DEC_PRIVATE_MODE_RESET, //implemented
CHARACTER_ATTRIBUTES, //implemented
SET_OR_RESET_RESOURCE_VALUES,
DEVICE_STATUS_REPORT,
DISABLE_MODIFIERS,
DEVICE_STATUS_REPORT_DEC,
SET_RESOURCE_VALUE_POINTER_MODE,
SOFT_TERMINAL_RESET,
REQUEST_ANSI_MODE,
REQUEST_DEC_PRIVATE_MODE,
REQUEST_DEC_PRIVATE_MODE, //implemented
SET_CONFORMANCE_LEVEL,
LOAD_LEDS,
SET_CURSOR_STYLE,
Expand Down Expand Up @@ -338,16 +343,16 @@ public class TerminalStream : Object {
INSERT_COLUMNS,
DELETE_COLUMNS,

SET_TEXT_PARAMETERS,
SET_TEXT_PARAMETERS, //implemented

FTCS_PROMPT,
FTCS_COMMAND_START,
FTCS_COMMAND_EXECUTED,
FTCS_COMMAND_FINISHED,
FTCS_TEXT_MENU_START,
FTCS_TEXT_MENU_END,
FTCS_PROGRESS,
FTCS_EXECUTE_COMMANDS
FTCS_PROMPT, //implemented
FTCS_COMMAND_START, //implemented
FTCS_COMMAND_EXECUTED, //implemented
FTCS_COMMAND_FINISHED, //implemented
FTCS_TEXT_MENU_START, //implemented
FTCS_TEXT_MENU_END, //implemented
FTCS_PROGRESS, //implemented
FTCS_EXECUTE_COMMANDS //implemented
}

// TODO: Use accessor methods ("add_parameter()") instead of public(?)
Expand Down Expand Up @@ -427,10 +432,13 @@ public class TerminalStream : Object {
add_esc_sequence_pattern(ControlSequenceType.FORWARD_INDEX, "9");
add_esc_sequence_pattern(ControlSequenceType.APPLICATION_KEYPAD, "=");
add_esc_sequence_pattern(ControlSequenceType.NORMAL_KEYPAD, ">");
add_esc_sequence_pattern(ControlSequenceType.INDEX, "D");
add_esc_sequence_pattern(ControlSequenceType.NEXT_LINE, "E");
add_esc_sequence_pattern(ControlSequenceType.CURSOR_TO_LOWER_LEFT_CORNER_OF_SCREEN, "F");
add_esc_sequence_pattern(ControlSequenceType.FULL_RESET, "c");
add_esc_sequence_pattern(ControlSequenceType.MEMORY_LOCK, "l");
add_esc_sequence_pattern(ControlSequenceType.MEMORY_UNLOCK, "m");
add_esc_sequence_pattern(ControlSequenceType.REVERSE_INDEX, "M");
add_esc_sequence_pattern(ControlSequenceType.INVOKE_G2_CHARACTER_SET_AS_GL, "n");
add_esc_sequence_pattern(ControlSequenceType.INVOKE_G3_CHARACTER_SET_AS_GL, "o");
add_esc_sequence_pattern(ControlSequenceType.INVOKE_G3_CHARACTER_SET_AS_GR, "|");
Expand Down