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

Added support for z/OS. #3289

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ require (
github.com/dustin/go-humanize v1.0.0
github.com/go-errors/errors v1.0.1
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51
github.com/mattn/go-isatty v0.0.11
github.com/mattn/go-isatty v0.0.16
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, due to the fact that it was introduced with mattn/go-isatty@360bbd2 in v0.0.13 first.

Copy link
Author

Choose a reason for hiding this comment

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

Do you think I should change it to v.0.0.13? I didn't see that in some list I saw somewhere.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Usually I've no problem to update to the latest tag, but above v0.0.17 we would loose the documented go compatibility of v1.16 due to the dependency of golang.org/x/sys. At least as far as I understood.

Copy link
Collaborator

Choose a reason for hiding this comment

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

After the update you should squash your last 5 commits and correct the commit description, since you switched from v0.0.16 to v0.0.13.

Copy link
Author

Choose a reason for hiding this comment

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

I'm not positive, but I think I've done this. (I'm not a power user of git.)

github.com/mattn/go-runewidth v0.0.7
github.com/mitchellh/go-homedir v1.1.0
github.com/sergi/go-diff v1.1.0
Expand All @@ -14,7 +14,7 @@ require (
github.com/zyedidia/clipper v0.1.1
github.com/zyedidia/glob v0.0.0-20170209203856-dd4023a66dc3
github.com/zyedidia/json5 v0.0.0-20200102012142-2da050b1a98d
github.com/zyedidia/tcell/v2 v2.0.10 // indirect
github.com/zyedidia/tcell/v2 v2.0.10
Copy link
Collaborator

Choose a reason for hiding this comment

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

Slightly unrelated to the PR, right?

Copy link
Author

Choose a reason for hiding this comment

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

I don't think I made this change. I think the build did. Does it have an effect?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest to stay with the current content, as long as it isn't really needed here.

@dmaluka:
What's your opinion about it, since you're more familiar in go than me.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not really familiar with this particular aspect of Go. But I agree it doesn't look like we should change this tcell line.

github.com/zyedidia/terminal v0.0.0-20230315200948-4b3bcf6dddef
golang.org/x/text v0.3.8
gopkg.in/yaml.v2 v2.2.8
Expand Down
8 changes: 0 additions & 8 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,6 @@ github.com/zyedidia/json5 v0.0.0-20200102012142-2da050b1a98d h1:zmDMkh22zXOB7gz8
github.com/zyedidia/json5 v0.0.0-20200102012142-2da050b1a98d/go.mod h1:NDJSTTYWivnza6zkRapeX2/LwhKPEMQ7bJxqgDVT78I=
github.com/zyedidia/poller v1.0.1 h1:Tt9S3AxAjXwWGNiC2TUdRJkQDZSzCBNVQ4xXiQ7440s=
github.com/zyedidia/poller v1.0.1/go.mod h1:vZXJOHGDcuK08GXhF6IAY0ZFd2WcgOR5DOTp84Uk5eE=
github.com/zyedidia/tcell/v2 v2.0.9 h1:FxXRkE62N0GPHES7EMLtp2rteYqC9r1kVid8vJN1kOE=
github.com/zyedidia/tcell/v2 v2.0.9/go.mod h1:i4NNlquIQXFeNecrOgxDQQJdu+7LmTi3g62asvmwUws=
github.com/zyedidia/tcell/v2 v2.0.10-0.20221007181625-f562052bccb8 h1:53ULv4mmLyQDnqbjVxanckP57WSreWHwTmlLJrJEutY=
github.com/zyedidia/tcell/v2 v2.0.10-0.20221007181625-f562052bccb8/go.mod h1:i4NNlquIQXFeNecrOgxDQQJdu+7LmTi3g62asvmwUws=
github.com/zyedidia/tcell/v2 v2.0.10-0.20230320201625-54f6acdada4a h1:W4TWa++Wk6uRGxZoxr2nPX1TpIEl+Wxv0mTtocG4TYc=
github.com/zyedidia/tcell/v2 v2.0.10-0.20230320201625-54f6acdada4a/go.mod h1:i4NNlquIQXFeNecrOgxDQQJdu+7LmTi3g62asvmwUws=
github.com/zyedidia/tcell/v2 v2.0.10-0.20230831153116-061c5b2c7260 h1:SCAmAacT5BxZsmOFdFy5zwwi6nj1MjA60gydjKdTgXo=
github.com/zyedidia/tcell/v2 v2.0.10-0.20230831153116-061c5b2c7260/go.mod h1:i4NNlquIQXFeNecrOgxDQQJdu+7LmTi3g62asvmwUws=
github.com/zyedidia/tcell/v2 v2.0.10 h1:6fbbYAx/DYc9A//4jU1OeBrxtc9qJxYCZXCtGQbtTWU=
github.com/zyedidia/tcell/v2 v2.0.10/go.mod h1:i4NNlquIQXFeNecrOgxDQQJdu+7LmTi3g62asvmwUws=
github.com/zyedidia/terminal v0.0.0-20230315200948-4b3bcf6dddef h1:LeB4Qs0Tss4r/Qh8pfsTTqagDYHysfKJLYzAH3MVfu0=
Expand Down
2 changes: 1 addition & 1 deletion internal/action/actions_posix.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// +build linux darwin dragonfly solaris openbsd netbsd freebsd
// +build linux darwin dragonfly solaris openbsd netbsd freebsd zos
Copy link
Collaborator

Choose a reason for hiding this comment

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

What about...

// +build linux darwin dragonfly openbsd_amd64 freebsd
&
// +build !linux,!darwin,!freebsd,!dragonfly,!openbsd_amd64

...?

Hm curious, solaris and netbsd aren't treated here?!

Copy link
Author

Choose a reason for hiding this comment

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

@JoeKar, yeah I'm not sure exactly what is going on there. The only use of RunTermEmulator I could find is in the fzf plugin (https://github.com/micro-editor/updated-plugins/blob/master/fzf/main.lua). It checks for shell.TermEmuSupported and if run invokes shell.RunTermEmulator, otherwise invokes shell.RunInteractiveShell. And if shell.RunInteractiveShell fails it invokes (local) fzfOutput.

The plugin works without any source change. I think it's working with RunInteractiveShell. But I really don't understand why.

If I add zos to build for terminal_supported.go and !zos for terminal_unsupported.go I get "Unsupported operating system". Which makes no sense to me, since that message is in terminal_unsupported.go, which it shouldn't be building.

Since it is working with no changes I figured I'd deal with it later. But if you have any ideas, that would be great.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's relevant here...

if !TermEmuSupported {

...and here too...
if !TermEmuSupported {

Have you really build with GOOS being set to zos? According to...
https://go.dev/doc/install/source#environment
...it's isn't really documented, but in...
https://github.com/golang/go/blob/f43d9c40f382def04442898d7581402759bff36a/src/go/build/syslist.go#L32
...it's listed, so I assume it should work in the moment it's invoked with the correct GOOS environment variable for a fitting architecture.


package action

Expand Down