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

fix broken completion of screen on osx, test on ubuntu and mac #3271

Merged
merged 6 commits into from
Aug 19, 2016

Conversation

arkbriar
Copy link
Contributor

@arkbriar arkbriar commented Jul 28, 2016

Description

Change sed expression of function '__fish_complete_screen' in share/completion/screen.fish to support completion on both mac and linux.
I have tested on my mac and ubuntu server.

Fixes issue #

fish does not complete screen -r when tapping Tab on mac (both GNU and FAU version)

Output example

on mac:

~/W/G/f/s/completions (master)$ screen -r 819
81930.test1  (Screen: Detached)  81951.test3  (Screen: Detached)
81940.test2  (Screen: Detached)  81961.test4  (Screen: Detached)

on linux (ubuntu):

xxx@localhost ~> screen -r 4
4171.test1  ((07/28/16 08:09:41) Screen:Detached)  4272.test3  ((07/28/16 08:10:19) Screen:Detached)
4255.test2  ((07/28/16 08:10:17) Screen:Detached)  4289.test4  ((07/28/16 08:10:21) Screen:Detached)

You will find that they are a little different because screen -list on mac doesn't show time.

@faho
Copy link
Member

faho commented Jul 28, 2016

Can you post an example of the output? Ideally, this'd be switched over to our string tool. I haven't done so so far because I couldn't properly test it, since I don't use screen.

@floam floam changed the title fix broken completion of screen on osx, test on ubuntu and mac with f… fix broken completion of screen on osx, test on ubuntu and mac Jul 28, 2016
@floam
Copy link
Member

floam commented Aug 1, 2016

Cool.

In context though, before you type the first "4" in examples at the top, these appear along with many other completions. On Linux, the date means these no longer fit in a single column and you have to scroll down through more pages to get to them:
screenshot 2016-08-01 at 1 47 53 am
screenshot 2016-08-01 at 1 39 41 am

The omitted space after "Screen:" is kinda ugly. I think I'd rather drop the "Screen:" here than add it though. These columns in the pager can be narrow. Maybe it could be -r32189.pts-3.aaron (08/01/2016 08:45:00 AM attached)?

I wonder what we can do to make it less of a pain to scroll past all the other options upon screen -r<tab>. It'd be nice if they appeared at the front or were suggested right after a screen <tab> instead of the files in the current directory it can't do anything with.

@floam
Copy link
Member

floam commented Aug 1, 2016

Hm, one can adjust LC_TIME to perhaps get a more narrow format/ISO 8601 through en_DK:

screen -list
There are screens on:
    32189.pts-3.aaron   (08/01/2016 08:45:00 AM)    (Attached)
…

begin; set -lx LC_TIME en_DK.UTF-8; screen -list; end
There are screens on:
    32189.pts-3.aaron   (2016-08-01 08:45:01)   (Attached)
…
begin; set -lx LC_TIME POSIX; screen -list; end
There are screens on:
    32189.pts-3.aaron   (08/01/16 08:45:01) (Attached)

@floam
Copy link
Member

floam commented Aug 2, 2016

Wow!

@floam floam self-assigned this Aug 2, 2016
function __fish_detect_screen_socket_dir --description "Detect which folder screen uses"
set screen_bin screen
if not set -q __fish_screen_socket_dir
set -g __fish_screen_socket_dir (eval $screen_bin -ls __fish_i_don_t_think_this_will_be_matched | string match -r "(?<=No Sockets found in ).*(?=\.)")
Copy link
Member

Choose a reason for hiding this comment

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

Looking over Apple's released screen code, while the per-user TMPDIR thing is an Apple addition, I don't think it's the case that only Macs will have an old screen with no date output (I could be wrong?).

I worry checking for this specific string might break for localizations. Could we try to detect the path whether or not there were attached screens? -q -list and exit status might help but probably isn't necessary.

Copy link
Contributor Author

@arkbriar arkbriar Aug 2, 2016

Choose a reason for hiding this comment

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

The socket directory is a compiler option so we could't know the path of a screen binary before it is run. Also, TMPDIR is used by the builtin screen on mac (not those installed with brew which use /tmp/uscreens/S-$USER, not even others). So I have to tell which path current screen command of current user is using If we want to steal the time from those socket files.

And we also could not know whether a screen on other platform shows time or not before there are sessions.

-q -ls can only tell whether there are sockets matched in the SockDir.

-list Do nothing, just list our SockDir [on possible matches].

-q Suppress printing of error messages. In combination with "-ls" the exit value
is as follows: 9 indicates a directory without sessions. 10 indicates a direc-
tory with running but not attachable sessions. 11 (or more) indicates 1 (or
more) usable sessions.

So as far as I see, the only way to get the sockdir is to run screen -ls.

Localization is a big problem. However, think about it: they could translate "Detached" to "分离" while they translate "No Socket in ..." into "没有..." . We can not prevent anyone from doing this.
So I think it is not strictly necessary to consider localization.

@arkbriar
Copy link
Contributor Author

arkbriar commented Aug 2, 2016

Setting LC_TIME doesn't change the output of screen on my ubuntu server. Maybe some compiler options were not opened to support this.

begin; set -lx LC_TIME POSIX; screen -list; end
There are screens on:
    30035.test  (08/01/16 21:49:14) (Detached)
begin; set -lx LC_TIME en_DK.UTF-8; screen -list; end
There are screens on:
    30035.test  (08/01/16 21:49:14) (Detached)

They just behave the same way.


I use a trick for screen on mac to detect the time and status. Let me show you how it works.

I have these screen sockets in socket directory.

/p/t/u/S-xxx$ ll
total 0
srw-------  1 xxx  staff     0B Aug  2 09:52 90828.name
srw-------  1 xxx  staff     0B Aug  2 09:12 90843.test
srw-------  1 xxx  staff     0B Aug  2 09:12 90861.so
srw-------  1 xxx  staff     0B Aug  2 09:14 91053.fdsjk
srw-------  1 xxx  staff     0B Aug  2 09:14 91087.tes

They are all detached.

There are screens on:
    90828.name  (Detached)
    90843.test  (Detached)
    90861.so    (Detached)
    91053.fdsjk (Detached)
    91087.tes   (Detached)
5 Sockets in /tmp/uscreens/S-xxx.

And then I change the file permission of one of them.

/p/t/u/S-xxx$ chmod 700 90843.test
/p/t/u/S-xxx$ ll
total 0
srw-------  1 xxx  staff     0B Aug  2 09:52 90828.name
srwx------  1 xxx  staff     0B Aug  2 09:12 90843.test
srw-------  1 xxx  staff     0B Aug  2 09:12 90861.so
srw-------  1 xxx  staff     0B Aug  2 09:14 91053.fdsjk
srw-------  1 xxx  staff     0B Aug  2 09:14 91087.tes
/p/t/u/S-xxx$ screen -ls
There are screens on:
    90828.name  (Detached)
    90843.test  (Attached)
    90861.so    (Detached)
    91053.fdsjk (Detached)
    91087.tes   (Detached)
5 Sockets in /tmp/uscreens/S-xxx.

The 90843.test is attached now without executing screen -r. So I think that screen uses the execute permission to tell whether one socket is in use, at least on mac.

Thus I change the list function for mac by using stat command to get the time, name and status.

It relies on stat and can not distinguish irrelevant files. :(

I'm sorry I have no time to come up with another good idea.

stat -f "%Lp %Sm %N" -t "%D %T" $__fish_screen_socket_dir/* | string match -r '^7\d{2} .*$' | string replace -r '^7\d{2} (\S* \S*) \S*/(\S+)' '$2\t$1 Attached'
pushd $__fish_screen_socket_dir > /dev/null
ls
set sockets (ls)
Copy link
Member

Choose a reason for hiding this comment

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

set -l

@floam floam merged commit 044efef into fish-shell:master Aug 19, 2016
@faho faho added this to the next-2.x milestone Aug 27, 2016
@krader1961 krader1961 modified the milestones: fish 2.4.0, next-2.x Sep 3, 2016
nomaed pushed a commit to nomaed/fish-shell that referenced this pull request Jul 17, 2017
…shell#3271)

* fixes broken completion of screen on osx, test on ubuntu and mac with fish 2.3.1
* replaces sed, __fish_sgrep with fish builtin string
* add completion for `screen -x`
* adjust format (e.g. 12345.socket\t01/01/16 09:55:00 Detached)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants