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

Remove fish_greeting.fish from this prompt #175

Open
neon64 opened this issue Jun 5, 2019 · 12 comments
Open

Remove fish_greeting.fish from this prompt #175

neon64 opened this issue Jun 5, 2019 · 12 comments
Labels
❓discussion understanding the project and its choices 🚀 enhancement performance, UX or maintainability
Milestone

Comments

@neon64
Copy link

neon64 commented Jun 5, 2019

Installing this prompt overrides my own fish_greeting.fish file. While I understand its nice to get rid of the default fish greeting, shouldn't that be the responsibility of the user or a separate plugin, and not the responsibility of a prompt?

In other words, I feel it should be possible to mix and match prompts and greetings, rather than one controlling the other.

On another note: thanks very much for this prompt - I particularly like how the arrow direction switches with fish's vim mode - that's much more subtle than any mode indicator I had devised!

@edouard-lopez edouard-lopez added the ❓discussion understanding the project and its choices label Jun 5, 2019
@edouard-lopez
Copy link
Member

Hello @neon64,
thanks for the feedback.

Was you fish_greeting.fish located in $HOME/.config/fish/functions/fish_greeting.fish? That's where installer place our fish_greeting.fish file, its the recommended way to leverage autoload mechanism.

Solution

You can either remove $HOME/.config/fish/functions/fish_greeting.fish and fallback to your setup or copy your setup in it.

Discussion

I understand your point about responsability but pure vision is to provide a clean and pure experience of the prompt. So, removing greeting and other presets sounds in line with this.

However, one is free to customize upon it as described above.

Cheers!

@wilfredjonathanjames
Copy link

Bumping.

Why is pure the plugin that gets to overwrite the fish_greeting.fish? What if other plugins want to do the same?

You're overwriting it because you want to check for updates, but shouldn't that be up to the user? Most fish users would be happy to update their greeting to add that feature.

Every time I run my fisher installer, this plugin wipes out my greeting. You're not really leaving much room for other people to customise fish.

@edouard-lopez
Copy link
Member

Thanks for the feedback, do you think I can automate the addition of the call?

@edouard-lopez edouard-lopez reopened this Sep 2, 2022
@edouard-lopez edouard-lopez added the 🚀 enhancement performance, UX or maintainability label Sep 2, 2022
@wilfredjonathanjames
Copy link

I would definitely consider adding it to the README as an option to add to config.fish - I personally wouldn't add it. I like to update my plugins manually.

@jorgebucaran
Copy link
Contributor

Here's an alternative. Instead of shipping an empty fish_greeting function, Pure could just set -U fish_greeting "" after install. This will suppress Fish's default greeting (in line with @edouard-lopez's line of thought) without wiping out an existing user's custom fish_greeting function. 🤓

@edouard-lopez
Copy link
Member

edouard-lopez commented Sep 28, 2022

Current Behaviour

Currently, on new session, pure checks for new release and show command to install, as we override the fish_greeting.fish file.

Proposed Behaviour

⚠️ Introduce a breaking change in pure's behaviour that will require user action.

  • Stop overriding user functions/fish_greeting.fish
  • Document how to add the new release check
  • Suppress Fish's default greeting, as suggested by @jorgebucaran
  • Provide instructions to stdout on install as suggested by @wjagodfrey

Does that sound right, to you?

@wilfredjonathanjames
Copy link

wilfredjonathanjames commented Oct 1, 2022

@edouard-lopez that sounds right to me. You could also follow the pattern of extending the file if it exists, or the config.fish, or provide instructions to stdout on install as you sometimes see with apt/brew

@jorgebucaran
Copy link
Contributor

Suppress Fish's default greeting, as suggested by @jorgebucaran

There are various ways one may accomplish this. I would define an install event handler in conf.d/pure.fish like so:

function _pure_install --on-event pure_install
    set -U fish_greeting ""
end

@edouard-lopez
Copy link
Member

edouard-lopez commented Oct 4, 2022

You could also follow the pattern of extending the file if it exists,

How would you do that programmatically?

provide instructions to stdout on install

That's a good idea!

@wilfredjonathanjames
Copy link

Just bumped into this again. @edouard-lopez I agree option 2 is cleaner. Would you like to do it or do you want me to have a go? If you point me to the piece of code that does the overwrite I can take a shot.

@edouard-lopez
Copy link
Member

edouard-lopez commented Jan 16, 2023

@jorgebucaran I defined a custom fish_greeting function in the project. I believe @wjagodfrey own file is being overwritten during install. Emptying fish_greeting variable won't resolve the issue when people have added a custom fish_greeting function.

However, we can:

  1. rename the project's fish_greeting.fish to _pure_fish_greeting.fish so it doesn't override user file ;

  2. add a post_install event handler that check the situation so that:

    • when fish_greeting.fish is missing, use our (i.e. rename _pure_fish_greeting.fish as fish_greeting.fish) ;

    • when fish_greeting.fish already exists, output instruction to add call to _pure_check_for_new_release:

      echo "_pure_check_for_new_release" >> $fish_function_path/fish_greeting.fish
      

@wjagodfrey I'm open to PR, here are some entry points:

The project has a docker container you can use (cf. makefile)

@jorgebucaran
Copy link
Contributor

My suggestion is to simply remove fish_greeting.fish and use the install event exactly as I suggested in #175 (comment). That's all you need to do.

@edouard-lopez edouard-lopez added this to the ⏩ v4.0-next milestone Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
❓discussion understanding the project and its choices 🚀 enhancement performance, UX or maintainability
Projects
None yet
Development

No branches or pull requests

4 participants