Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add psuedo terminal support #6

Closed
wants to merge 7 commits into from
Closed

Conversation

bluetech
Copy link
Contributor

@bluetech bluetech commented Jan 9, 2012

Hi,
So I felt like plugging a real shell might be the next logical step. I know you're working on this too, but its pretty straightforward so I did it myself. If you haven't got something like this locally, then have a look :)

The first four commits are pretty simple preparatory stuff.
The final commit makes vte.c handle all the pty stuff on its own and report to the terminal object when necessary. I had to make some changes along the way, hopefully they are fine.

Also, I wasn't entirely sure what your intentions were regarding unicode handling and which subsystem should do what, specifically the utf-8 conversion. So I just ignored it here for now, but it's mostly orthogonal to this series.

Thanks,
Ran

[Added a couple of commits that slipped me yesterday, specifically I forgot to waitpid().]

Add KMSCON_ERR, the equivalent of EPOLLERR.

Signed-off-by: Ran Benita <ran234@gmail.com>
This makes things easier in the common case of
<some error> -> <log error> -> <goto error handler> - <do something with
errno>

Signed-off-by: Ran Benita <ran234@gmail.com>
The other layouts can be confusing.

Signed-off-by: Ran Benita <ran234@gmail.com>
These are the only open() calls missing the flag.

Signed-off-by: Ran Benita <ran234@gmail.com>
This commit adds some needed terminal emulator infrastructure.

We allow to "open" (and close) a vte object (resp. terminal object).
We then create a pty pair, fork and exec a shell. We route input to the
shell and draw its output to the console.

We add callbacks for when
- The buffer changes (through the vte object). We can then schedule a
  screen redraw.
- The shell (child process) exits. We can then exit ourselves, start up
  a new shell, etc.

There is not yet any real VTE processing, so we display raw escape
codes and so on. However, this should provide immediate feedback for
any further vte development, as we start to act like a real terminal
emulator.

Signed-off-by: Ran Benita <ran234@gmail.com>
Unfortunately, there is no clean way I see to hook this up from the vte
object. We can (and will) have more than one vte object opened at a
time, but the semantics of signalfd make it impossible to deliver each
SIGCHLD to its rightful owner without complicating things.

[ From what I tested:
 - If you have two signalfd's listening to the same signal, they will be
   dispatched in some round-robin manner.
 - Also, if more than one child exits before we read signalfd (possibly
   beloging to different terminals), they will be compressed to one
    event. ]

We therefore need to do the reaping from a central location, and need to
remember to copy this snippet over to main.c in the future.

Signed-off-by: Ran Benita <ran234@gmail.com>
We do actually set it, but we don't really have to. Protect it just in
case.

Signed-off-by: Ran Benita <ran234@gmail.com>
@dvdhrm
Copy link
Owner

dvdhrm commented Jan 11, 2012

I have picked up the miscellaneous fixes but I am not very happy with the pty implementations. Why don't you put the pty API in a different file (vte_pty.c or so)? Use a kmscon_pty structure and this will then be used by the vte object. I don't like using the vte structure for pty handling. I will add some more inline comments tomorrow but I want to finish my OpenGL to OpenGLESv2 conversion first.

It is also much easier for me to review patches if they are split up into logical patches and not just one big patch. This means using many "git rebase"s during development, but if the patches are clean and small, the code will always be bisectable, stable and reviewable.

Cheers
David

@dvdhrm dvdhrm closed this Jan 11, 2012
@dvdhrm dvdhrm reopened this Jan 11, 2012
@@ -95,7 +121,37 @@ void kmscon_vte_bind(struct kmscon_vte *vte, struct kmscon_console *con)
kmscon_console_ref(vte->con);
}

void kmscon_vte_input(struct kmscon_vte *vte, kmscon_symbol_t ch)
/* FIXME: this is just temporary. */
void kmscon_vte_input(struct kmscon_vte *vte, struct kmscon_input_event *ev)
Copy link
Owner

Choose a reason for hiding this comment

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

We shouldn't expose kmscon_input_event here. First, kmscon_symbol_t was introduced to be used as internal representation of input, second, we must convert XK_Return to \n in the input system so copy/paste will also work correctly. This is the wrong place to handle this. We should rely on kmscon_symbol_t solely here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to close this pull request and send out a V2.

I have to disagree with this comment though. The vte subsystem needs the full event, at least for the following reasons:

  • It needs to know the modifier state (e.g. for control characters).
  • It needs to handle keysyms which are not translatable to unicode (e.g. arrow keys).
  • The interpretation may depend on the vte state (e.g. NL/LF mode as one example).
  • And generally the keyboard interpretation we talk about should be done in the terminal emulator because it is logically, well, terminal emulation.

I consider kmscon_symbol_t to be essentially a wchar_t of sorts with the extra ability to hold combining characters (through the symbol table). This should only be the business of the console subsystem, however.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants