JanDeDobbeleer / oh-my-posh

The most customisable and low-latency cross platform/shell prompt renderer
https://ohmyposh.dev
MIT License
16.53k stars 2.32k forks source link

Export-PoshTheme disregards the -FilePath parameter #748

Closed skycommand closed 3 years ago

skycommand commented 3 years ago

Prerequisites

Description

The Export-PoshTheme seems to have trouble understanding the relative path in the -FilePath parameter. Instead of resolving the relative path according to the current folder, it resolves the path according to the desktop!

Environment

Steps to Reproduce

  1. Launch PowerShell 7.1 via pwsh.exe -noprofile
  2. Issue the following commands, one by one:
Set-Location 'D:\Projects\AdminScripts\' #or another directory of your choosing
Import-Module -Name 'oh-my-posh' -MinimumVersion 3.0
Set-PoshPrompt Slim
Export-PoshTheme -FilePath '.\slim.json' -Format json

Expected behavior: slim.json must appear in D:\Projects\AdminScripts

Actual behavior: slim.json appears in ~ or (in one occasion) ~\Desktop (Use a utility like Everything to locate it in real-time)

lnu commented 3 years ago

If you don't set the FilePath, the text is copied to the clipboard.
From what I see, the FilePath works well with a fully qualified path but it has an issue with relative paths.

JanDeDobbeleer commented 3 years ago

@lnu this is going to be a fix like mapping HOME

lnu commented 3 years ago

I did this:

else {
    $FilePath = "$(pwd)\$FilePath"
}

after the

 if ($FilePath.StartsWith('~')) {
     $FilePath = $FilePath.Replace('~', $HOME)
}

Final code:

    if ($FilePath -ne "") {
        if ($FilePath.StartsWith('~')) {
            $FilePath = $FilePath.Replace('~', $HOME)
        }
        else {
            $FilePath = "$(pwd)\$FilePath"
        }
        [IO.File]::WriteAllLines($FilePath, $configString)
    }
    else {
        Set-Clipboard $configString
        Write-Output "Theme copied to clipboard"
    }

It seems ok

JanDeDobbeleer commented 3 years ago

@lnu won't that fail when you add a full path?

lnu commented 3 years ago

Yep, we should add a check to see if the path is full with ‘IsPathRooted’(for example). Or use GetFullPath. I don’t remember if it will resolve \~.

JanDeDobbeleer commented 3 years ago

@lnu I guess we can consider this closed now, right?

lnu commented 3 years ago

For me yes, can you validate @skycommand ?

skycommand commented 3 years ago

@lnu and @JanDeDobbeleer Since you said that the problem has been fixed, I took the pain of testing it on both my machine and a clean virtual machine. In both tests, I installed Oh-my-posh fresh from the Internet. In both cases, version 3.158.4 got installed.

I am afraid I can say with certainty that the bug persists. On my virtual machine, Export-PoshTheme attempted to write the file into my Windows folder!

Actual machine's screenshot: Actual machine

Virtual machine's screenshot: Virtual machine

JanDeDobbeleer commented 3 years ago

@skycommand "you took the pain"? Really? That's how we're addressing people who took up the task the same day waiting on your feedback to see if it was also resolved on your end?

I'll check later today, in the meanwhile, work on your attitude and gratitude.

skycommand commented 3 years ago

@JanDeDobbeleer It's an expression. "Took the pain" means "carried out a certain task". No actual pain is involved. Sorry, that's how English works.

lnu commented 3 years ago

Yes, There is a bug for powershell core reported about this(I found it on macos this morning). The fix is to pass $pwd as second parameter to GetFullPath. https://github.com/PowerShell/PowerShell/issues/10278

skycommand commented 3 years ago

Oh, yeah. PowerShell's run space cannot alter the PowerShell host process's current directory. There is a reason for that: In run space, the current directory is a PSDrive not an actual file system volume. Consider this:

Set-Location HKLM:\Software
$PWD.ProviderPath

These paths are not even on the file system.

github-actions[bot] commented 4 months ago

This issue has been automatically locked since there has not been any recent activity (i.e. last half year) after it was closed. It helps our maintainers focus on the active issues. If you have found a problem that seems similar, please open a discussion first, complete the body with all the details necessary to reproduce, and mention this issue as reference.