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

Use 'UnixMode' on *nix platforms instead of Windows-only 'Mode' #98

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ShrykeWindgrace
Copy link

Description

I had to increase the width of the Mode colum to 10 (was 7), otherwise it wraps. We can not add a switch on the OS in the definition of this width, the XML schema would not allow that; we could create format on the fly, but that seems like significant refactoring with possible impact on gci's latency.

Otherwise, the change is a simple switch over the OS to select an appropriate property of the file to be rendered.

Related Issue

#66

Motivation and Context

It allows showing *nix file permissions on non-windows platforms.

How Has This Been Tested?

Tested on my Debian. The tests run by ./build.ps1 are all green, the imported module shows unix permissions/windows permissions depending on the platform.

Screenshots (if appropriate):

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.

@ShrykeWindgrace
Copy link
Author

@devblackops A gentle reminder =)

@ShrykeWindgrace
Copy link
Author

@devblackops Another ping=)

@devblackops
Copy link
Owner

@ShrykeWindgrace Sorry for not responding earlier. It looks like there are potential variations on whether pwsh includes a Mode or UnixMode property on Linux. I'm testing with WSL2 and pwsh 7.1.3 and the property is Mode, not UnixMode like you have in the PR. Perhaps instead of checking for $isWindows, it should be whether a Mode or UnixMode property exists and use whatever is available.

@ShrykeWindgrace
Copy link
Author

@devblackops Hm, that's a good point. I'll check what is available where, since we can have a pure windows setup, a pure *nix setup, a WSL1 looking into a WSL file and into a windows file, and then again there's WSL2 looking into its own files or into windows files... I have a suspicion that we will be hitting the limitations of what is possible with *.format.ps1xml.

I'll keep you posted.

@ShrykeWindgrace
Copy link
Author

I ran my tests to cover the combinatorics. Summary: I do not have a valid Mode whenever I run pwsh from a WSL[1/2].

Config: pwsh-7.3.4 on windows side installed from MS Store, pwsh-7.3.2 on different WSLs installed via nix.

origin of pwsh and of the file Win WSL1 WSL2
file inside WSL FS did not test Mode: 👎 , UnixMode: ✔️ Mode: 👎 , UnixMode: ✔️
file outside WSL FS Mode: ✔️ , UnixMode: 🔴 Mode: 👎 , UnixMode: ✔️ Mode: 👎 , UnixMode: ✔️

I do not have access to a native linux environment today, but I am confident that Mode will be unset =)

There might have been a change of pwsh's behavior between your version and mine. If that's indeed the case, the support of different platforms with different pwsh versions... That would quickly become a hassle.

I'll to get a 7.1.3 on my WSLs (no ETA).

@ShrykeWindgrace
Copy link
Author

On a 7.1.3 the situation is even funnier. Despite the documentation insisting on the presence of UnixMode in non-windows environments (https://learn.microsoft.com/en-us/previous-versions/powershell/module/microsoft.powershell.management/get-childitem?view=powershell-7.1), this property does not exist at all in my tests - in other words, I confirmed your results. Again, I take a pwsh-7.1.3 from nix on a WSL2.

That being said, 7.1.X is old - the LTS is 7.2, if I remember correctly; and current is 7.3.4.

Anyways, one could consider a different approach altogether - instead of having one-size-fits-all.format.ps1xml, implement several ones, and depending on pwsh's version and and OS choose an appropriate one. I have an impression that that format file does not allow for a lot of customization, switches, and tinkering.

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.

None yet

2 participants