Skip to content

Conversation

scribu
Copy link
Member

@scribu scribu commented Jun 24, 2013

Follow-up to #548.

When using man, the output was paged. You could achieve the same effect, without this patch, by running this:

wp help core config --color | less -r

less bug

When I run the above command, the output skips the "NAME" header.

Omitting the --color flag has no effect.
Changing from iTerm2 to Terminal has no effect.
Piping explicitly instead of from PHP has no effect.

Removing config from the command does have an effect. Maybe it's because the synopsis for core config is longer than $COLUMNS.

@scribu
Copy link
Member Author

scribu commented Jun 24, 2013

@Rarst: could you give this a spin?

@Rarst
Copy link
Contributor

Rarst commented Jun 24, 2013

Works with less provided by Git, but fails for Windows native more:

Notice: Undefined offset: 1 in C:\server\www\wp-cli\php\commands\help.php on line
68
Warning: stream_get_contents() expects parameter 1 to be resource, null given in C:\server\www\wp-cli\php\commands\help.php on line 68

@scribu
Copy link
Member Author

scribu commented Jun 24, 2013

That means that more fails for some reason (and that I got the wrong pipe handle).

You should be able to see the error message by piping it explicitly (using the master branch):

wp help core config --color | more -r

@Rarst
Copy link
Contributor

Rarst commented Jun 24, 2013

No issue with explicitly piping it to more, other than it doesn't have -r argument, but removing it from code doesn't fix issue on pager branch.

Something probably goes bonkers in proc_open() (common story on win) I will see if I can debug it later today.

@Rarst
Copy link
Contributor

Rarst commented Jun 25, 2013

Looked closer at it. Actually it tries to run less -r and never reaches more because

    if ( 127 == $r ) {
        return false;
    }

doesn't trigger. $r is int(1) in this case, whatever that is supposed to mean...

more works if I change $pagers = array( 'less -r', 'more -r' ); to $pagers = array( 'more' );

@scribu
Copy link
Member Author

scribu commented Jun 25, 2013

127 is the exit code bash sends if a command is not found. It seems to be the same on Windows: http://msdn.microsoft.com/en-us/library/windows/desktop/ms681382%28v=vs.85%29.aspx#ERROR_PROC_NOT_FOUND

@Rarst
Copy link
Contributor

Rarst commented Jun 25, 2013

Error code meaning might be same, but I am not getting that error code, it's 1 (ERROR_INVALID_FUNCTION according to that list) instead.

@scribu
Copy link
Member Author

scribu commented Jun 25, 2013

Ok, thanks. I guess I'll just have to fire up my old Windows laptop one of these days and figure it out.

@scribu
Copy link
Member Author

scribu commented Aug 4, 2013

It seems detecting if a command is available is not so straigthforward: http://superuser.com/questions/175466/determine-if-command-is-recognized-in-a-batch-file/175831#175831

Also, I found that the list command is a closer equivalent to less: http://malektips.com/xp_dos_0021.html (but it's not available by default)

* Windows throws an unexpected exit status when trying to use `less`
* more doesn't have an '-r' flag
@scribu
Copy link
Member Author

scribu commented Aug 4, 2013

Decided to skip paging on Windows, since I don't have a Windows machine handy to test either way.

Manual piping should work, though: wp help core config | more.

scribu pushed a commit that referenced this pull request Aug 4, 2013
Make `wp help` use a pager
@scribu scribu merged commit 19d8f27 into master Aug 4, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants