matchai / spacefish

πŸš€πŸŸ The fish shell prompt for astronauts
https://spacefish.matchai.dev
MIT License
962 stars 79 forks source link

Create spacefish hostname function #49

Closed Snuggle closed 6 years ago

Snuggle commented 6 years ago

Here's my attempt at converting https://github.com/denysdovhan/spaceship-prompt/blob/master/sections/host.zsh to fish. There may be some mistakes, since I'm quite new to fish. Fixes issue #20.

There are probably issues with this and I'd like to ask for help for integrating + testing. I'm not sure how this is supposed to be done.

Description

I've attempted to convert host.zsh into fish. I'm not quite sure how to integrate/test these changes in Spacefish, however, so there's a high possibility of changes needing to be made.

Motivation and Context

Attempts to solve #20 and allows people to see their hostname while SSHing into another machine.

Types of changes

Screenshots:

image

How Has This Been Tested?

I have run echo statements to ensure the script only runs while SSH is active. I'm not sure how to make the variables available to Spacefish or how Spacefish sources the function. Script shows errors when being run directly, since spacefish::section doesn't appear to exist.

image Very loosely tested using debug echo statements. Not sure if colors work. End represents the end of the file. Is running represents the main condition that checks if an SSH connection is active, passes. image

I would like to ask for help testing these changes and making sure they integrate correctly into spacefish. Feel free to make any changes. :fish:

Default Pull Request Checklist:

Personal Development Checklist:

matchai commented 6 years ago

Hey Snuggle! Thank you for getting started on #20! Here are a couple things that will help you get on your way.

Here is where the sections are imported. All sections are prefixed by __sf_section_ before they're ran, so you will want to add your section to the list, and rename the file and function to match the formatting.

I would take a look at the following section file to use as reference for code formatting, setting defaults and getting spaceship::section function (__sf_lib_section in spacefish) to work: https://github.com/matchai/spacefish/blob/master/functions/__sf_section_user.fish

Looks like you're on the right track! Don't hesistate to ask if you need help with any specifics! πŸ˜„ Thanks again for the help!

Snuggle commented 6 years ago

@matchai Could you please give me some pointers on how to test my code, both locally and with your automated ci-testing? I have experience with Travis, but none with WIP or appveyor.

I need to add some checks to my tests. I'd like to test both in and without an SSH connection, as well as testing that changing the variables functions correctly, but I don't know how to add these tests.

test "Correctly shows hostname upon SSH connection"
    (
        ssh localhost
        # How to add a check here?

    ) = (__sf_section_host)
end

(Apologies for the three unsigned commits, I derped git signing. :yum:)

Snuggle commented 6 years ago

I'm not sure why the automated tests are failing on a test that I didn't create. :slightly_frowning_face:

image

test "Don't truncate path"
    (pwd) = (
        __sf_util_truncate_dir (pwd) 0
    )
end
matchai commented 6 years ago

Looks like you're making a lot of headway! πŸ˜„

To run the tests locally, you should be able to simply run ./tests/run.fish from the project root. That should create a fresh fish and spacefish installation and run the tests.

Travis and Appveyor should automatically run, and should simply run the same way as it would locally (/tests/run.fish). Travis running on Mac and Linux, and Appveyor running on Cygwin on Windows. WIP simply makes the PR unable to merge until "[WIP]" is removed from its title.

Regarding the random failure; after some tinkering, it looks like it was caused by the unescaped single-quote on this line (not sure why it's getting caught up on a comment). πŸ˜…

The tests are ran with fishtape, which has some documentation, but you might benefit more from referencing some of the pre-existing tests in the repo.

Snuggle commented 6 years ago

@matchai

image image

How is this snippet of code testing the user section, though? It's echoing "with snuggle", but my prompt says "in snuggle", how is this test passing?

"with snuggle" != "snuggle in"

➜ ssh localhost
Last login: Wed Jul 25 10:31:39 2018 from 192.168.0.101
Welcome to fish, the friendly interactive shell

snuggle in ~ 
➜ 

image I haven't changed the behavior in my fish config either.

matchai commented 6 years ago

Good question! πŸ˜„

The "with" is the prefix to the user section, while the "in" is the prefix to the dir section.

The prefix of the first section to be printed is hidden by default because of the following config: https://github.com/matchai/spacefish/blob/master/fish_prompt.fish#L10

You can see that in action here: image

In the section integration tests, we are running them in isolation, so the prefixes will be shown.

Snuggle commented 6 years ago

This should be complete, hopefully!

I had to remove SPACEFISH_HOST_SHOW_FULL as a config option, as it uses Zsh-only functionality that doesn't seem to exist in Fish, but it doesn't seem very useful anyways.

Permalink to Zsh's function

I've added six eight tests total, which should test everything within this section. I can only locally test for Windows + Linux, so if you're able to test it'd be appreciated. :slightly_smiling_face:

It might be worth closing this pull request and waiting on #52.

matchai commented 6 years ago

I would also suggest moving the following line from "upcoming" to "features": https://github.com/matchai/spacefish/blame/master/README.md#L39

matchai commented 6 years ago

Looks squeaky clean to me! πŸ› We're off to the races!