matchai / spacefish

🚀🐟 The fish shell prompt for astronauts
https://spacefish.matchai.dev
MIT License
960 stars 78 forks source link

Ensure functionality of spacefish on the 3.0 release of fish #128

Closed matchai closed 5 years ago

matchai commented 5 years ago

Description

All the following changes should be backward compatible with fish 2.7.1:

Motivation and Context

Closes #127

Types of changes

How Has This Been Tested?

Checklist:

matchai commented 5 years ago

I'm having a heck of a time trying to figure out what is causing this failure. This would be a whole lot easier if the test runner would escape all backslash characters and color codes so that we can more easily debug 🤔 Edit: Oh! We totally can by looking at the travis log!

I was thinking it had to do with the new fish_ambiguous_width and fish_emoji_width variables, but messing with those locally still causes the same failures.

matchai commented 5 years ago

After a little bit of chatting on the fish-shell gitter, this bit of advice might help us isolate the failure: image

matchai commented 5 years ago

Here's the rest of the conversation, where @faho discusses the solution he found: image

The failure was resolved in the end with the following PR to fishtape: https://github.com/jorgebucaran/fishtape/pull/35

matchai commented 5 years ago

Hmm... 🤔 Now we've got a new failure to do with identifying the root of a git repo. This also doesn't happen outside of tests and only seems to affect the Travis run.

matchai commented 5 years ago

I've gone ahead and removed Appveyor and Cygwin-related testing from the project. Appveyor has been consistently triggering false positives which are not reproducible on my local Cygwin installation, causing us to add logic specifically for Appveyor test runs.

I'll look into re-adding Cygwin (and possibly WSL) coverage with Travis in a future PR. 👍

matchai commented 5 years ago

So it seems... 🤔 It appears our tests are still running correctly, regardless. 🤷‍♂️

matchai commented 5 years ago

:tada: This PR is included in version 1.12.4 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

faho commented 5 years ago

builtin -n, though that lists all builtins, even the ones you are allowed to override, like cd.

Am Mi., 9. Jan. 2019, 11:17 hat Snuggle notifications@github.com geschrieben:

@Snuggle commented on this pull request.

In tests/mock.fish https://github.com/matchai/spacefish/pull/128#discussion_r246326251:

@@ -18,7 +18,7 @@ #

function mock -a cmd -a argument -a exit_code -a executed_code -d "Mock library for fish shell testing"

  • set -l cmd_blacklist "builtin" "functions" "eval" "command"
  • set -l cmd_blacklist "builtin" "functions" "eval" "command" "argparse" "read" "set" "status" "test"

This seems like a pain in the butt to maintain as things are added over time. Can fish do this for us? Is there a command to list all builtins automatically?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/matchai/spacefish/pull/128#discussion_r246326251, or mute the thread https://github.com/notifications/unsubscribe-auth/AE8fVx6YEIeQwYzn2FhvyVOGiJ26oMnGks5vBcGdgaJpZM4ZkF3r .

Snuggle commented 5 years ago

@matchai Could we possibly replace this list of builtins with the above? It seems like it would be much easier to maintain.

matchai commented 5 years ago

As part of #136, the mock code is no longer in the repo. In fish-mock, I've gone ahead and added the full list. 😄