Jaykul / PowerLine

A more PowerShell prompt
MIT License
567 stars 31 forks source link

Enh req - API tweaks #5

Closed rkeithhill closed 8 years ago

rkeithhill commented 8 years ago

I think the Prompt and Line types should use aggregation rather than inheritance. That will make the Intellisense completion list way more relevant and the object model becomes more intuitive IMO:

$PowerLinePrompt.Lines[0].Blocks[0]
$PowerLinePrompt.Lines[1].Blocks.Insert(0, ....)

Also, rather than rely on position relative to the [BlockCache]::Column object why not either have a Block property Alignment e.g.:

@{ bg = "DarkRed";  fg = "White"; align = "Right"; text = $Env:USERNAME + "@" + $Env:COMPUTERNAME}

I think that would make the alignment feature be a bit more discoverable. Another option would be to have two collections in Line - BlocksRightAligned and BlocksLeftAligned.

Jaykul commented 8 years ago

Interesting. Originally, I actually did have $Prompt.Lines[0].Blocks[0] but it felt complicated and wordy, particularly in construction. I wanted to be able to just cast the array of arrays for the constructor, and spent quite a bit of effort getting that right, and in the end I decided that I liked the brevity of $PowerLinePrompt[0][0] ...

I have considered that it might have been a mistake, because of how PowerShell unrolls collections, so if you do $PowerLinePrompt | Get-Member you don't see the features of the prompt, but of the lines. I've also considered that --now that I've worked out how to get the constructors right -- it doesn't matter whether the Line is a collection or has a collection, as long as I keep the constructors.

Having said that, it would be a breaking change at this point (however minor), and quite a bit of work, because if I did that, I would feel obliged to go back to using PowerShell classes, since the only reason I gave in and switched to C# is because generic collection inheritance of user-defined types doesn't work in PowerShell classes...

I would really like to know if anyone else has an opinion about this.

Jaykul commented 8 years ago

As far as alignment ... I thought of that, as well:

The original design goal was just "more than one column" -- that is, just multiple columns, in general, not just left and right, but also center, or multiple automatic width columns, so I could write a Format-PowerLine like Format-Table.

Since the console doesn't have right alignment, right alignment requires calculating the width of what you want to write, and then starting that many places from desired end (the console width).

The "PowerLine" concept uses full line-height separators and therefore requires knowing the background colors of the blocks on both sides. This means that having output be out of order would be a major pain: even with only two columns, having a align="right" in the middle of align="left" would mean needing to keep track of both the location and color of the last output in each column. Right?

So anyway, the current design deliberately makes that impossible (by requiring an array for each column). Since it doesn't yet support more than two columns, the output logic just calculates the length of the existing output, and then the length of the remaining output in order to start the right column in the right position.

Alternative suggestion

I have considered having a property on the Block called ColumnBreak (just like there is a Clear property) which would just indicate a break, but still require the items to be in order, and hypothetically allow support for more columns later...

Do you @rkeithhill think that would make this clearer (enough)?

Jaykul commented 8 years ago

As a side-note, on the topic of design ...

I am currently doing a little more work to improve the ability of the value of a "block" to be a scriptblock which actually outputs blocks, e.g.:

@{ bg = "DarkRed";  fg = "White"; text = { Get-GitStatusPowerline }}

The idea is that if I have a line like this:

    @(
        @{ bg = "Blue";     fg = "White"; text = { $MyInvocation.HistoryId } }
        @{ bg = "Cyan";     fg = "White"; text = { [PowerLine.Block]::Gear * $NestedPromptLevel } }
        @{ bg = "Cyan";     fg = "White"; text = { if($pushd = (Get-Location -Stack).count) { "$([char]187)" + $pushd } } }
        @{ bg = "DarkCyan";               text = { Get-GitStatusPowerline } }
        @{ bg = "DarkBlue"; fg = "White"; text = { $pwd.Drive.Name } }
        @{ bg = "DarkBlue"; fg = "White"; text = { Split-Path $pwd -leaf } }
    )

When I process it, the first thing I do is "cache" everything. Currently, I'm doing that by casting each Block to a BlockCache (with pre-calculated output text). My idea for change is to alter this slightly so that if the object has a scriptblock definition, I'd invoke that, and create a BlockCache for the output, such that if the output is a Block, I would process that, recursively.

The idea is that it would flatten the output, so after caching, we might have, for instance (assuming we're not nested or push'd):

    @(
        @{ bg = "Blue";     fg = "White"; text = 5 }
        @{ bg = "DarkCyan"; fg = "White"; text = "master" } 
        @{ bg = "Cyan";     fg = "Black"; text = "+2 ~3" } 
        @{ bg = "DarkBlue"; fg = "White"; text = "C" }
        @{ bg = "DarkBlue"; fg = "White"; text = "WindowsPowerShell" }
    )
rkeithhill commented 8 years ago

ColumnBreak

That would be OK especially if the auto-completion list for Block was a more manageable size. :-)

Jaykul commented 8 years ago

I think I've dealt with all of this now...

I went with $PowerLinePrompt.Lines[0].Columns[0].Blocks and for now, the second column always goes on the right (and if you have more than two, the third one wraps to a new line).

You can leave a column empty using $null ...

rkeithhill commented 8 years ago

Cool.