Page MenuHomePhabricator

The function phutil_console_get_terminal_width needs to work with Windows
Open, Needs TriagePublic

Description

The phutil_console_get_terminal_width() function in the format.php file has a comment:

//TODO: Figure out how to make this work with Windows

I can get this to work with Windows (all flavors XP and above).

Event Timeline

RyanNerd created this task.Sep 18 2015, 9:02 PM
RyanNerd updated the task description. (Show Details)
RyanNerd added a project: libphutil.
RyanNerd added a subscriber: RyanNerd.
joshuaspence renamed this task from The function phutil_console_get_terminal_width() needs to work with Windows to The function phutil_console_get_terminal_width needs to work with Windows.Sep 19 2015, 3:56 AM
joshuaspence updated the task description. (Show Details)
preg_match('/CON.*:(\n[^|]+?){3}(?<cols>\d+)/', `mode`, $matches);
$cols = $matches['cols'];

This outputs the number of columns of the terminal window according to http://stackoverflow.com/questions/263890/how-do-i-find-the-width-height-of-a-terminal-window (it's a weird trick),so I assume the width can be concluded in some way from the number of columns. Tested it ,but however when resizing the window by mouse the number of columns stays the same,don't know whether this behaves exactly as the non-Windows implementation.

In the code the unix Command tput cols is used. All this really does is look at the $TERM environment variable and infers the column count from that. On my Mint 17.2 this is 'xterm' and the tput cols command always returns 80 (which is the default column width for xterm).

If I open a terminal and resize it then execute tputs cols in the terminal it will report back the column count of the resized terminal instance.

The mode command behaves similarly and will report the column width of the current command window.

phutil_console_get_terminal_width instantiates a new terminal or command window (otherwise null is returned; since the $TERM environment variable does not exist outside of a terminal window).

So even in Linux this function returns the default column width for the default terminal emulator (almost always 80).
In Windows, the mode command does the same thing returning the column width of the instantiated command window which by default for all flavors of Windows is 80.

Here is the code snippet for Unix/Linux:

  $tmp = new TempFile();

  // NOTE: We can't just execute this because it won't be connected to a TTY
  // if we do.
  $err = phutil_passthru('tput cols > %s', $tmp);

  if ($err) {
    return null;
  }

  try {
    $cols = Filesystem::readFile($tmp);
  } catch (FilesystemException $ex) {
    return null;
  }

  $width = (int)$cols;
}

I think my suggested code from stackoverflow for Windows behaves like its unix counterpart,as it outputs the same numberm,namely 80, and does not rely on the actual size of the terminal window.However,note,that I have not tested the following code in conjunction with phutil,only tested the code stated in my comment above.

So my suggestion thus far(or would it be better to submit this as a patch as it's apprently the usual workflow or not):
replace

if (phutil_is_windows()) {
     // TODO: Figure out how to get this working in Windows.
     return null;
   }

with

 if (phutil_is_windows()) {
     preg_match('/CON.*:(\n[^|]+?){3}(?<cols>\d+)/', `mode`, $matches);
     if(is_array($matches))&&isset($matches['cols'])){
return (int)$matches['cols'];
}else{
      return null;
}
    }

If you submit this as a differential revision it will give a better interface for others to review and discuss the suggested change.

I find it difficult to understand the regex -- it's always one of those things that I have to research regex for myself anytime I come across it. It would be beneficial for myself if you could describe it's intended behavior. I'm guessing it's trying to grab the 3rd line after the CON: is matched, and match the numbers on the line into a 'cols' key in the result.

Something that might make it simpler for parsing the output, I believe you can use mode con instead of just mode in order to specify you want status just for the console/keyboard device and not all devices. See:
https://technet.microsoft.com/en-us/library/bb490932.aspx

+1 vote from me to comment the use of regular expressions (how can anything that looks like this "'/CON.*:(\n[^|]+?){3}(?<cols>\d+)/" be called regular?)

The regular expression suggested by @theunreplicated does include the 'con' part of the the mode command. (e.g. 'mode con' or 'mode CON' will work as Windows is not as case sensitive as much as Linux is.)

My suggestion was regarding the

`mode`

changing to

`mode con`

Which will limit the output of the mode command to only include the keyboard/console device, rather than running a blanket mode that returns status for all devices, then relying on the regex to locate the CON. I suggested this as a way that might reduce the complexity of the regex.

"I see" said the blind man as he tripped over his code.

As for the regex note that I am not the author of it,as stated in my comment above(maybe it has been overlooked because i didn't say that clearly),i have just taken it from stackoverflow(if that's ok) to provide a solution for this task as it seemed to be straightforward.As you said this regex looks really weird,I for myself would't even be able to form such an regex,but I am able to explain roughly in detail.
So mode contains in my case 2 blocks,one with sth others and the other with the headline containng CON,this regex looks for the first appearance of CON followed by an arbitary number of any characters following a : .As you have pointed it's better to use the con directly via mode con,i have added a patch that does it.The : is follwed by a sequence marked by ( ),this sequence is detected when a line break(\n occurs) followed by an arbitary number of characters without |,so every time a line break occurs it will trigger that sequence.THe 3 in the { specifies how many times this sequence should match in the text before the regex engine proceeds with the next statement(I bet you're aware of this),which is (?<cols>\d+).
I guess ?<cols> is to copy the matched contents of this sequence in to the key with cols in $matches,so it's redundant as basically every sequence marked with( is accessible by a number as key.The \d stands for digits,so as soon as the regex engine encounters a non-digit,it will abort.

Suggestions:

preg_match('/(\n[^|]+?){4}(?<cols>\d+)/', `mode con`, $matches);

Not beginnig with CON will add up another line break,so replace 3 with 4.?<cols> is redundant so my recommended solution:

preg_match('/(\n[^|]+?){4}(\d+)/', `mode con`, $matches);

and access the value therefore with $matches[2] and check with array_key_exists instead of isset beforehand.

OK?

If you aren'T already aware of it here's ow the output of mode con looks like(changed the first from German to English):

Status von Ger„t CON:
---------------------
   Rows:          25
    Columns:         80
    Wiederholrate:   31
    Verz”gerungszeit:1
    Codepage:        850

"

I hope my explanation clarifies what this regex does.

I forgot about that. The Windows

mode

command will return the words such as "Columns" per the locale of Windows. So looking for literally "Columns" will only work for the English Windows locales.