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

Add Set-TerminalIconsIcon, Get-TerminalIconsGlyphs #35

Merged
merged 1 commit into from
Apr 21, 2021

Conversation

tillig
Copy link
Contributor

@tillig tillig commented Apr 20, 2021

Description

Get-TerminalIconsGlyphs allows users to see the available glyph set for easier theme customization. It also allows argument completion over the set of available glyphs.

Set-TerminalIconsIcon allows users to update an existing file name, file extension, or directory name to use a different icon. It also allows users to "swap" one glyph for another in the current theme, which allows streamlined theme customization.

Related Issue

Partially addresses #34 - allows users to more easily customize a theme to change icons that don't render properly in their given terminal.

Motivation and Context

Creating a new theme is somewhat challenging and requires a non-trivial amount of spelunking to figure out the right structure for the files/directories, the available set of glyphs, etc. These changes can help with that.

How Has This Been Tested?

Testing environment:

  • Mac OS Big Sur 11.2.3
  • PowerShell 7.1.3
  • iTerm2
  • Fira Code Nerd Font

Get-TerminalIconsGlyphs was tested locally and is also part of the argument completer for glyph parameters in Set-TerminalIconsIcon.

Set-TerminalIconsIcon was tested locally using all parameter sets against the default devblackops theme. Argument completers were also validated.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Notes:

  • It doesn't seem there's a good pattern for testing changes to the current theme. I didn't try to invent that pattern, but if there's a suggestion I'm glad to add some Pester tests.
  • This doesn't include the ability to modify the symbolic link icons for either files or directories. I thought it was getting a little cluttered. It could easily be added later.
  • I didn't add the ability to remove an icon from the current theme. I figured that might be a Remove-TerminalIconsIcon command, which could be added later. However, you can use the Set command here to add new ones.
  • On build I get a warning from the PSScriptAnalyzer about needing to include a UTF BOM in the .psm1 file. I'll include that below. I didn't modify the .psm1 file so I assume this has something to do with the build and possibly Mac OS. I did notice on clone that the .psm1 and .psd1 do not have a BOM; perhaps a .gitattributes update and an .editorconfig could help.

Warning from PSScriptAnalyzer:

RuleName                      Severity ScriptName          Line Message
--------                      -------- ----------          ---- -------
PSUseBOMForUnicodeEncodedFile Warning  Terminal-Icons.psm1      Missing BOM encoding for non-ASCII encoded file
                                                                'Terminal-Icons.psm1'

Get-TerminalIconsGlyphs allows users to see the available glyph set for
easier theme customization. It also allows argument completion over the
set of available glyphs.

Set-TerminalIconsIcon allows users to update an existing file name,
file extension, or directory name to use a different icon. It also
allows users to "swap" one glyph for another in the current theme,
which allows streamlined theme customization.
@tillig
Copy link
Contributor Author

tillig commented Apr 20, 2021

Seems the build is breaking with:

At /home/runner/work/Terminal-Icons/Terminal-Icons/Output/Terminal-Icons/0.4.0/Terminal-Icons.psm1:1184 char:36 + $_.Value | Export-Clixml -Path $colorThemePath -Force + ~~~~~~~~~~~~~~~ [<<==>>] Exception: The running command stopped because the preference variable "ErrorActionPreference" or common parameter is set to Stop: Cannot convert 'System.Object[]' to the type 'System.String' required by parameter 'Path'. Specified method is not supported.

The code in question is:

$userThemeData.Themes.Color.GetEnumerator().ForEach({
    $colorThemePath = Join-Path $userThemePath "$($_.Name)_color.xml"
    $_.Value | Export-Clixml -Path $colorThemePath -Force
})

...which isn't something I messed with.

It may relate to another thing I noticed when I first installed the module, which is that when I imported the module to start with I got a bunch of errors; after restarting PowerShell it was OK. I loosely guessed it had something to do with writing some files out to a folder during module install.

Any ideas on how I can fix it?

@devblackops
Copy link
Owner

Thanks @tillig. I'll dig into this PR.

Re: the build errors. Silly mistake on my part when creating the user theme directory and output multiple things to the output stream. Fixed in 91887c8.

@devblackops
Copy link
Owner

@tillig I think this looks good as-is. I don't have a pattern for testing theme changes (tests need to be improved in general). I can see a Set-TerminalIconsColor working the same way but that can be added separately. I like the fact that this only changes the applied theme in the session and doesn't write back to the theme directory. I suppose we can add support for that later, but I'd want to come up with some type of override system to the base theme isn't modified.

@devblackops devblackops merged commit 86d26af into devblackops:master Apr 21, 2021
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.

2 participants