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

Rename tabs to match the repo and branch when in powershell ise #605

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

Conversation

georgecheyne
Copy link

I'm not sure if you want to embed IDE specific code in here at all. If you do this also probably isn't the best place.
However I find this useful. One of the main reasons I use posh-git is because I can use it in Powershell ISE and use the MDI to have a few repos available at once.
I thought I'd share the code and see if you like it/want to incorporate it in some way.
Cheers,
George

@dahlbyk
Copy link
Owner

dahlbyk commented Aug 17, 2018

I've not heard of using ISE for the MDI…interesting. I am certainly not opposed to making like better for folks using ISE, especially since it's cheap to skip for non-ISE sessions.

What's the relationship between ISE tabs and $Host.UI.RawUI.WindowTitle?

I have some thoughts in general, but for starters can you move this new code into Set-WindowTitle and --amend that change into the existing commit? Would like to avoid the extra churn in Git prompt.ps1.

@rkeithhill
Copy link
Collaborator

In addition to moving to Set-WindowTitle should this feature be behind another GitPromptSettings property e.g. EnableIseTabTitle or something like that?

@georgecheyne
Copy link
Author

Hi guys,

Thanks for the feedback.

Maybe I should make another CmdLet called Set-TabTitle and call it within Set-WindowTitle is called.
I could encapsulate the code for creating the title and allow people who use other IDEs to pass in a callback to allow them to set the title on their IDE.

With the GitPromptSettings should it default to on or off? I would say this functionality would be desired by defualt for MDI IDEs.

I'll have a crack at the changes when I get some time and you can review.

I think the tabs and title are managed seperately.
It looks like the title reflects the repo [branch] where the last git command was run.

ise-tabs

I have noticed some behaviour that might freak people out a bit. If you have 2 tabs pointing to the same repo and you change branch in one it'll obviously change the branch in the other tab but that won't be reflected in the ide until you run another git command. It makes sense when you know how git works but though I'd point it out.

 Changes to be committed:
	modified:   src/GitPrompt.ps1
		remove the old code
	modified:   src/PoshGitTypes.ps1
		add config entry to disable tab labelling
	modified:   src/WindowTitle.ps1
		added functions to label tabs
	modified:   src/posh-git.psm1
		added call to tab labelling code
@@ -273,6 +273,8 @@ class PoshGitPromptSettings {
[string]$DescribeStyle = ''
[psobject]$WindowTitle = {param($GitStatus, [bool]$IsAdmin) "$(if ($IsAdmin) {'Admin: '})$(if ($GitStatus) {"$($GitStatus.RepoName) [$($GitStatus.Branch)]"} else {Get-PromptPath}) ~ PowerShell $($PSVersionTable.PSVersion) $([IntPtr]::Size * 8)-bit ($PID)"}

[bool]$TabTitle = $true
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should consider following the pattern that $WindowTitle uses. By specifying a scriptblock, users will be able to easily customize the text that gets put in the tab title. To simplify the scriptblock, you can use functions. Perhaps a Get-UniqueTabTitle to handle the logic for appending a number to the title to make it unique. Perhaps it would be something like this:

[psobject]$TabTitle = {param($GitStatus, [bool]$IsAdmin) "if ($GitStatus) { Get-UniqueTabTitle "$($GitStatus.RepoName) [$($GitStatus.Branch)]" }}

}

# If the user is running Powershell ISE then name the tab
if($psISE -and $GitStatus){
Copy link
Collaborator

Choose a reason for hiding this comment

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

The code style for this project uses a single space after keywords like if, foreach, etc and a single space before an opening {.

@@ -78,6 +78,7 @@ $GitPromptScriptBlock = {

# This has to be *after* the call to Write-VcsStatus, which populates $global:GitStatus
Set-WindowTitle $global:GitStatus $IsAdmin
Set-TabTitle $global:GitStatus
Copy link
Collaborator

Choose a reason for hiding this comment

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

Even if folks don't use $IsAdmin in their customized scriptblock, I think we should pass it. No harm in having this extra bit of info injected into the scriptblock.

@@ -58,3 +58,45 @@ function Set-WindowTitle {
}
}
}

function Set-TabTitle {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we change the setting to be a scriptblock, then this function becomes very similar to the Set-WindowTitle function above except that where it sets the title, it uses $psise.CurrentPowerShellTab.DisplayName = …

}
}

function Get-TabTitle {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we change this to Get-UniqueTableTitle with a single parameter of $Title and this function simply ensures that $Title is unique and if not, it will append numbers until a unique title is found.

Copy link
Owner

@dahlbyk dahlbyk Aug 18, 2018

Choose a reason for hiding this comment

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

👍

I think you can simplify a bit, too:

function Get-UniqueTabDisplayName ($Name, $Format="{0} {1}") {
  $tabNumber = 1
  $names = $psISE.PowerShellTabs | Select-Object -ExpandProperty DisplayName
  $newName = $Name
  while ($names -Contains $newName) {
    $newName = $Format -f $Name,$tabNumber++
  }
  return $newName
}

Note I dropped the parentheses to align with ISE's default numbering scheme.

Copy link
Author

Choose a reason for hiding this comment

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

I think I tried it like this initially but noticed an edge case where if you open a few tabs to the same repo i.e. posh-git [master], posh-git [master] (1), posh-git [master] (2) but then you close tab posh-git [master] (1) and open a new tab at the same repo it tries to create a new tab called posh-git [master] (2) and fails. Hence the logic to get the highest number that has been assigned so far.

Copy link
Owner

Choose a reason for hiding this comment

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

I believe this correctly fills in the gaps:
image
image
image

That said, this does open the possibility of changing branches/tabs such that you get:

[ "posh-git [master]", "posh-git [master] (1)", "posh-git [master] (2)"]
[ "posh-git [master]", "posh-git [george]", "posh-git [master] (2)"]
[ "posh-git [master]", "posh-git [george]", "posh-git [george] (1)"]
[ "posh-git [george] (2)", "posh-git [george]", "posh-git [george] (1)"]

As opposed to:

[ "posh-git [master]", "posh-git [master] (1)", "posh-git [master] (2)"]
[ "posh-git [master]", "posh-git [george] (1)", "posh-git [master] (2)"]
[ "posh-git [master]", "posh-git [george] (1)", "posh-git [george] (2)"]
[ "posh-git [george]", "posh-git [george] (1)", "posh-git [george] (2)"]

Copy link
Author

Choose a reason for hiding this comment

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

You're right this is much better and simpler. I hadn't looked at your looping logic properly I thought it was simply counting the number of tabs with the same name and using that count to name the tab. Wrist slapped.
Personally, I don't really think it matters what the number is as long as it's unique and doesn't change for a given tab.

$tabName= "$tabName ($tabCount)"
}
return $tabName
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add a final newline to this file.

Copy link
Owner

@dahlbyk dahlbyk left a comment

Choose a reason for hiding this comment

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

@rkeithhill's feedback is all 💯

}
}

function Get-TabTitle {
Copy link
Owner

@dahlbyk dahlbyk Aug 18, 2018

Choose a reason for hiding this comment

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

👍

I think you can simplify a bit, too:

function Get-UniqueTabDisplayName ($Name, $Format="{0} {1}") {
  $tabNumber = 1
  $names = $psISE.PowerShellTabs | Select-Object -ExpandProperty DisplayName
  $newName = $Name
  while ($names -Contains $newName) {
    $newName = $Format -f $Name,$tabNumber++
  }
  return $newName
}

Note I dropped the parentheses to align with ISE's default numbering scheme.

@georgecheyne
Copy link
Author

Thanks for all the feedback guys. I'll try to get it all cleaned up. I don't use powershell that much is there something like FxCop for powershell so I can make sure I conform to your coding standards?

@rkeithhill
Copy link
Collaborator

@georgecheyne If you use Visual Studio Code and install the PowerShell extension, you can select your code and press Ctrl+K, Ctrl+F to format the selected script.

	modified:   src/PoshGitTypes.ps1
		Made the TabTitle setting more like WindowTitle so it can be customised by users
	modified:   src/WindowTitle.ps1
		Removed tab handling code from here
	new file:   src/TabTitle.ps1
		moved all Tab related functionality here
		added code to reset tab names when leaving a repo space
		also will reset when unloading the posh-git module
		implemented simpler unique tab naming algorithm as suggested
	modified:   src/posh-git.psm1
		Added references to the new TabTitle script file and it's functions
@georgecheyne
Copy link
Author

georgecheyne commented Aug 19, 2018

Hi guys,

I've refactored it to make it much more like the window title implementation.

I think I've got the coding standards OK but any feedback welcome.

Cheers,

George

@dahlbyk
Copy link
Owner

dahlbyk commented Aug 19, 2018

Changes look good - I like how you've tried to make this not specific to ISE. However, I noticed my suggested Get-UniqueTabTitle ends up flipping the current tab back and forth between "posh-git [master]" and "posh-git [master] 1" (for example), since it's not aware of the current tab value.

I have noticed some behaviour that might freak people out a bit. If you have 2 tabs pointing to the same repo and you change branch in one it'll obviously change the branch in the other tab but that won't be reflected in the ide until you run another git command. It makes sense when you know how git works but though I'd point it out.

I also noticed this, too. If we're going to update the current tab, we might as well try to update other tabs for the current repo as well. I don't know exactly how it would work, but I wonder if we could use Add-Member to save each tab's working directory (or its $GitStatus? both?) on $psISE.CurrentPowerShellTab. Then each time we go to update the current tab DisplayName, we'd also update any tab for the current repo. We could also keep track of GitTabNumber and OriginalDisplayName for each tab, to make the experience even more seamless.

Thoughts?

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

Successfully merging this pull request may close these issues.

4 participants