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

[DO NOT MERGE} initial zsh support #355

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

[DO NOT MERGE} initial zsh support #355

wants to merge 8 commits into from

Conversation

detiber
Copy link
Contributor

@detiber detiber commented Sep 11, 2014

Changes introduced:

  • add zsh support to Terminal.vala
  • start of zsh_startup.in script
    • basically copied from bash_startup.in
      • bashisms removed
      • use function autoloading instead of bash specific export -f
  • Termlet support
    • separated out bash and zsh termlets
    • ls, ps, and wget termlets appear to be working correctly now for both bash and zsh

Outstanding issues:

  • history completion menus are not working for zsh
  • segfault on hitting enter on an empty line :(

@detiber detiber mentioned this pull request Sep 11, 2014
- wget - bottom of terminal is now updated when Download is initiated,
  and progress is updated, but it is not cleared when download
  finished/canceled

- ps - menus actually show up now
@detiber
Copy link
Contributor Author

detiber commented Sep 13, 2014

ls termlet is working as expected, ps termlet is now working as well.

wget termlet updates the status bar when download is initiated and when progress updates, but not when download is cancelled/finished.

history completion is not working at this time as well.

@detiber
Copy link
Contributor Author

detiber commented Sep 13, 2014

another major oddity right now is that the prompt is inheriting from the shell finalterm is launched from currently and not updating properly.

…ult setting (bash) if SHELL is not supported
@detiber
Copy link
Contributor Author

detiber commented Sep 14, 2014

Improved shell handling a bit, now attempt to consume $SHELL from environment and if supported, then use it otherwise fall back to the default value in Settings.get_default().shell_path


if (!(shell_basename in valid_shells)){
message(_("shell defined in environment is not supported, falling back to bash"));
shell = Settings.get_default().shell_path;
Copy link
Owner

Choose a reason for hiding this comment

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

If Settings.get_default().shell_path returns an invalid shell, we will proceed with that invalid shell from this point anyway. Instead, this should somehow "hardcode" bash so we have a true fallback.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true, I'll add logic to force bash if Settings.get_default().shell_path is not a valid shell. Thinking about it, I should also test to make sure that the shell referenced by $SHELL or shell_path actually exists as well.

@detiber
Copy link
Contributor Author

detiber commented Sep 17, 2014

I'm thinking there has got to be a better way to run configure_file on multiple files with cmake, but at least for the time being I'm defaulting to brute forcing it for the Termlet processing. I realized that different platforms might have different paths for the shells, so we should at least try to detect the shell path and set the #!'s appropriately.

@detiber
Copy link
Contributor Author

detiber commented Sep 17, 2014

I was able to fix up the issue with the wget termlet, but it introduced a crash when hitting at the prompt:
** (finalterm:20996): ERROR **: Highlight regex compilation error: Error while optimizing regular expression ➤
▶▶▶ 01:59.36 Wed Sep 17 2014 \xe2: internal error: opcode not recognized
zsh: trace trap (core dumped) finalterm

haven't had a chance to see if it is related to my zsh config or if it is a general issue yet.

@detiber
Copy link
Contributor Author

detiber commented Sep 17, 2014

actually, looking at the full output, it looks like my prompt is being interpreted by finalterm, will need to find a way to filter that out:

jdetiber@localhost:~➤ finalterm
** Message: TerminalOutput.vala:386: Command mode entered
** Message: Command updated: ''
** Message: Command updated: '\xa4 
'
** Message: Command updated: '\xa4 
\xe2'
** Message: Command updated: '\xa4 
\xe2'
** Message: Command updated: '\xa4 
\xe2\x96'
** Message: Command updated: '\xa4 
\xe2\x96'
** Message: Command updated: '\xa4 
▶'

...snip...


** Message: Command updated: '\xa4 
▶▶▶ 02:04.56 Wed Sep 17 2014 ◀◀◀                                                
jdetiber@localhost:~'
** Message: Command updated: '\xa4 
▶▶▶ 02:04.56 Wed Sep 17 2014 ◀◀◀                                                
jdetiber@localhost:~\xe2'

** (finalterm:22741): ERROR **: Highlight regex compilation error: Error while optimizing regular expression \xa4 
▶▶▶ 02:04\.56 Wed Sep 17 2014 ◀◀◀                                                
jdetiber@localhost:~\xe2: internal error: opcode not recognized
zsh: trace trap (core dumped)  finalterm
▶▶▶ 02:04.57 Wed Sep 17 2014 ◆ Exit Code: 133 ◀◀◀                                                                    
jdetiber@localhost:~➤ 

@p-e-w
Copy link
Owner

p-e-w commented Sep 23, 2014

What exactly is the content of your prompt? Is it

▶▶▶ 02:04.56 Wed Sep 17 2014 ◀◀◀                                                
jdetiber@localhost:~➤

?

@detiber
Copy link
Contributor Author

detiber commented Sep 23, 2014

It is indeed. But this type of prompt manipulation is common with zsh. I suspect it may just be a matter of escaping the prompt text somehow, but I haven't had a chance to dig into it yet.

zsh still does not invoke the menu, but at least now it doesn't crash on
no input
and an empty command.

Looks like the cursor_position is < command_start_postion (at least with
my .zshrc)
@harj0
Copy link

harj0 commented Jan 21, 2015

Sorry fur such a newb question, but how can I pull/implement these changes?

@Eugene-msc
Copy link

Like this i think
git branch zsh
git checkout zsh
git pull https://github.com/detiber/finalterm zsh2

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

Successfully merging this pull request may close these issues.

4 participants