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

implement a prompt_hostname function #3482

Merged
merged 3 commits into from
Oct 23, 2016
Merged

implement a prompt_hostname function #3482

merged 3 commits into from
Oct 23, 2016

Conversation

krader1961
Copy link
Contributor

Quick sanity check please that I didn't introduce a typo.

Standardize how the host name is included in the prompts that do so.

Fixes #3480

Standardize how the host name is included in the prompts that do so.

Fixes #3480
@faho
Copy link
Member

faho commented Oct 23, 2016

Did you forget to commit the function?

Copy link
Member

@floam floam left a comment

Choose a reason for hiding this comment

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

fish: Unknown command 'fish_prompt_hostname'

@floam
Copy link
Member

floam commented Oct 23, 2016

Should this maybe be prompt_hostname? Simply for discoverability, and to match prompt_pwd?

@krader1961
Copy link
Contributor Author

Did you forget to commit the function?

Oops, yes, stupid git commit -a doesn't include new files. Fixed.

@floam
Copy link
Member

floam commented Oct 23, 2016

Is splitting on '.' from hostname output always equivalent to using hostname -s?

(I genuinely have no idea.)

@krader1961
Copy link
Contributor Author

Should this maybe be prompt_hostname? Simply for discoverability, and to match prompt_pwd?

Agreed. Changed.

@krader1961
Copy link
Contributor Author

Is splitting on '.' from hostname output always equivalent to using hostname -s?

It's supposed to be since the definition of hostname -s is to convert a FQDN to a simple host name.

@floam
Copy link
Member

floam commented Oct 23, 2016

In no case can the FQDN be something like "www.mysite.com" and hostname -s and the simple host name be "myserver"? That'd be my only worry if it's possible, it'd now output "www".

@krader1961
Copy link
Contributor Author

If by simple host name you mean what is returned as the nodename by the uname(3) then "yes" in theory. In practice probably not because a) that value is supposed to be the network name and should match whatever gethostname() returns. If it doesn't the system is misconfigured and that isn't our problem.

@floam floam changed the title implement a fish_prompt_hostname function implement a prompt_hostname function Oct 23, 2016
@faho
Copy link
Member

faho commented Oct 23, 2016

Is splitting on ',' from hostname output always equivalent to using hostname -s?

First of all, that's what we currently do, so there's no regression here. Secondly, this allows us to fix it in one place if we ever figure out a better solution.

Thirdly: Probably not. This is probably one of those things where there's a super-rare super-complicated edge case that some people get really passionate about (maybe some way to make it recursive, or dependent on locale?). We'll get an issue and then we can work it out. Or they can just override the function.

@faho faho added this to the next-2.x milestone Oct 23, 2016
@floam
Copy link
Member

floam commented Oct 23, 2016

First of all, that's what we currently do, so there's no regression here

We used to use hostname -s here:

   # Just calculate these once, to save a few cycles when displaying the prompt
    if not set -q __fish_prompt_hostname
        set -g __fish_prompt_hostname (hostname -s)
    end

@faho
Copy link
Member

faho commented Oct 23, 2016

Yeah, but in the other prompts we didn't. This was inconsistent.

@krader1961 krader1961 merged commit 37d91d0 into fish-shell:master Oct 23, 2016
@krader1961 krader1961 deleted the fish-prompt-hostname branch October 23, 2016 23:50
@floam
Copy link
Member

floam commented Nov 1, 2016

It looks like most of the sample prompts are not using this.

@floam
Copy link
Member

floam commented Nov 1, 2016

Er, I'm totally mistaken. I was on Integration_2.4.0 and thought I was on master. And I mistakenly pushed two commits I shouldn't have. Reverting.

@faho faho added the release notes Something that is or should be mentioned in the release notes label Dec 10, 2016
elasticdog added a commit to elasticdog/dotfiles that referenced this pull request Feb 11, 2017
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cleanup enhancement release notes Something that is or should be mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sample fish prompts call hostname too often
3 participants