fish-shell / fish-shell

The user-friendly command line shell.
https://fishshell.com
Other
25.75k stars 1.9k forks source link

Not loading descriptions for functions in $fish_function_path #328

Open anbotero opened 11 years ago

anbotero commented 11 years ago

At first I thought it was just the functions I had in ~/.config/fish/functions, but I tried auto-completion suggestions by typing:

  va<Tab>

And noticed that there wasn’t a description for vared either, even though there was one when I checked functions vared.

The miscellaneous functions I load separately with . load their description just fine.

PS: I’m on OS X 10.8.2 (Mountain Lion), using fish OpenBeta_r2 version.

ridiculousfish commented 11 years ago

This is a longstanding bug present in the original fish. Since the completions infer the function name from the file, and doesn't actually load the function, so it can't produce the description.

frederickjh commented 8 years ago

I would also like to add that even if you use the temporary work-around in fish-shell/fish-shell#1915 posted by VosaXalo, creating a function with the same name as a command produces some interesting tab completion results.

Example: I have hub installed (the Github wrappper for git). In ~/.config/fish/functions/git.fish I have:

function git --description 'Alias for Github wrapper for git called hub, that adds some Github sugar to git.'
    hub $argv
end

Entering g Tab gives this: g_tab_result This is the description from the function.

Entering gi Tab gives this: gi_tab_result This is the description from the git man page.

Entering git Tab gives this: git_tab_result Just an executable without a description this time.

I am not sure what is going on here but something does not seem correct. I think that it should get the same description each time. I also think that the description of a function (an alias) with the same name as a command should be chosen over the description of the command itself.

floam commented 8 years ago

Could possibly be worked around by the manpage completions? Or use whatis for all commands unless overwritten?

herrbischoff commented 7 years ago

This is a longstanding bug present in the original fish. Since the completions infer the function name from the file, and doesn't actually load the function, so it can't produce the description.

I was wondering about this lately. Everywhere I looked I found mentions of --description 'whatever' but absolutely no mention of how to actually put it to use. So, to conclude: for the time being (and considering the age of this issue, probably for a long time), the --description flag is basically just an awkwardly placed comment and therefore useless, correct?

floam commented 7 years ago

For the time-being, it'd appear so. I'd like to see some way of making it useful though.

frederickjh commented 7 years ago

I would say that the description in a function in its current state only serves to remind the author when he edits it again as to what the function does. I just checked and now even @VosaXal workaround in my config.fish is no longer working. I am on fish version 2.6.0

herrbischoff commented 7 years ago

I would say that the description in a function in its current state only serves to remind the author when he edits it again as to what the function does.

Regular comments achieve this goal even better. If the description flag serves no practical purpose, I'd be in favor of either fixing it (finally) or removing it until it gets correctly implemented. The current state is highly confusing since users expect a present flag to actually do something.

frederickjh commented 6 years ago

My vote is for fixing it as it serves a useful purpose in allowing function authors to let people know what their function does or remind them if they have forgotten.

If removing comes up then I think that need to be thought over carefully. How will fish handle current functions that have a description? Will it ignore them or will this change break them?

+1 for fixing which is what this issue is about.

herrbischoff commented 6 years ago

@frederickjh: Thanks for the thumbs-down. It's amusing to see that you are under the illusion that you actually have say in this, given the age of the issue. You can "vote" all you like, as long as you don't provide the code, things will stay as they are.

What I wrote was simply that in any kind of code, a proper comment is better than something that looks like a useful but actually serves no purpose at all.

Example:

function foo --description 'whatever'
    doSomething
end

and

# whatever
function foo
    doSomething
end

and (if you care for the comment to be in the first line to be printable) even

function foo # whatever
    doSomething
end

are functionally eqivalent, which is confusing. Python developers are probably used to putting comments in weird places, like after instead of before the opening function declaration, the structure of the command (argument) however creates the expectation to somehow be able to use it, like being visible in the completion suggestions. Otherwise, what's the point.

I don't expect this to go anywhere, because if it did, it would already have. It will probably stay this way because the cost of making it work it apparently high, leaving it as is does not upset the "important function, do not remove" bickerers, although it is not a function and essentially does nothing already.

floam commented 6 years ago

Thanks for the thumbs-down. It's amusing to see that you are under the illusion that you actually have say in this, given the age of the issue.

Of course he has a say here, this is a public forum for fish nerds, the thread is open, we value different viewpoints.

Please try to show some basic courtesy to fellow contributors. Please argue in good faith, folks here are quite similar to you - we all want to improve Fish.

This is a volunteer project, we all love fish, let's try to remember most of us care a lot about how to make FIsh as good as it can be while we may disagree here and there, there is a lot of work to be done. We all want the best for fish, so let's try to focus on fish, not personal quibbles! Don't be mean to your comrades!

We all want to fix this problem, in an elegant way. That's the goal here and all are welcome to their input.

herrbischoff commented 6 years ago

Of course he has a say here, this is a public forum for fish nerds, the thread is open, we value different viewpoints.

I see what you mean and I can assure you it wasn't meant to be mean or disrespectful, just straight to the point. I value different viewpoints as well, as long as there are points to value. Which in this case is not quite so, in my view.

This issue has been open for 5+ years now with no signs of moving in any direction. That's why I assume while technically anybody who wants to has a "say" (insofar as they can say what they think) it does by no means translate into actually achieving anything by doing so.

You can "vote" all you like, as long as you don't provide the code, things will stay as they are.

I stand by this. The implicitness present in a statement like "I vote for doing/not doing ..." assumes that you believe it actually matters what you "vote for", even unsolicited, as in making things happen — in the absence of a logically coherent argument non the less. I guess especially the tone of "+1, me too!" has triggered me. +1-ing is a hollow gesture coming from a place of absolutely least effort. There are way too many of those flooding my inbox.

My vote is for fixing it as it serves a useful purpose in allowing function authors to let people know what their function does or remind them if they have forgotten.

See my previous response for an example why this is an invalid argument. There are already alternatives that are working now, are clearer and actually are comments that do nothing, as comments are expected to. In effect, nothing would be taken away. Comments have been used since forever to, well, comment code.

If removing comes up then I think that need to be thought over carefully. How will fish handle current functions that have a description? Will it ignore them or will this change break them?

This is a generic placeholder statement, which can be applied to literally anything. Also, it's invalid as a point because it does nothing more than stating the obvious.

+1 for fixing which is what this issue is about.

This is the kicker for me. +1 comments are generally frowned upon on GitHub (unless against all expectations you want to explicitly encourage them here), as they add no value but spam the inboxes of busy users. Also, as I previously mentioned, the other parts of the comment add no value as well. Combined with clicking the "thumbs-down" and "confused" emojis on my comment, this triggers my "bullshit warning" system. It's the Facebook-ization of everything I can't stand. Because clicking on a "like" button still does not a difference make, despite commonly held beliefs.

We want to fix this problem, in an elegant way.

That's good to hear. I'd be interested to hear any new info which hadn't been brought up in this issue before why exactly there has been zero progress on this in 5+ years and what the blockers with regard to this functionality are. I will be more than happy to join a discussion on this, which is my entire point and reason to subscribe to this issue in the first place.

faho commented 6 years ago

Soo... let's get back to business. I've implemented this now, in my funcdesc branch.

What that does is:

This causes a lot less slowdown than I thought - with 263 function files (i.e. overridden ones counted) I basically can't notice it. But my / is on an SSD (/home is on rotating rust), and my laptop is no slouch in general. I think I managed to spin up my fan with this, and it's on linux, which is generally rather good at caching, so I'd expect other machines with other drives and other OSen to perform worse.

I haven't specifically tried to optimize it (it might be a little faster to just read the first line of every file - my gut tells me that it's not much because that still needs to locate and open the file), and there's a crash that I've papered over that I don't quite get, but it's a nice conversation piece. I might print it out and hang it over the fireplace.

frederickjh commented 6 years ago

Thanks @faho! I appreciate your work on this despite the bikeshedding in this issue.

Regarding the slow down is this something that could configured so that it could be disable if the user has a computer with low resource where it would become a performance issue?


I do not really want to continue the bikeshedding, but I would like to make a few comments.

I never only post a +1 as my full comment to an issue. If I do so I always state my reasoning for saying why the issue is a valid issue.

As to producing code I have done this in the past with projects. Some code gets accepted some does not due to ideological differences in what is in the scope of a project.

I think this is why it is important to politely discuss the possible solution in the issue queue so that false starts in programming do not happen and waste programmer(s) resources, like time coding. For this reason now unless it is a simple syntax or typo error fix, I want to be sure that I am on the same page as the projects owners before submitting code.

zanchey commented 6 years ago

I don't think you can read just the first line of the file; that's not guaranteed to have the function of interest in it!

faho commented 6 years ago

I don't think you can read just the first line of the file; that's not guaranteed to have the function of interest in it!

@zanchey: Sure, yeah. But you could theoretically stop at the description. I don't think that's worth it, since it's probably not a massive gain performance-wise, and it's quite a bit of additional complexity.


One more thing I forgot to mention: This will source the file, which means it will execute any other code in there. That's probably unavoidable given that it's possible to conditionally-define functions - e.g. see our seq function in share/functions/seq.fish - but it might be surprising.

floam commented 6 years ago

I mentioned a couple ideas on Gitter. One was crazy, I posited storing descriptions in xattrs - that's a no go.

But I also wondered if it perhaps would be a good enough improvement for Fish to cache encountered descriptions somewhere, between runs, with descriptions being lazily updated? This wouldn't make every newly-encountered function immediately have a description at the ready but at least we wouldn't have think about how autoloading works or try solving the problem by changing how descriptions are defined.

frederickjh commented 6 years ago

I think that the caching idea is worth discussing. How does fish handle the descriptions for shell commands? Does it load these on the fly or cache them somewhere?

floam commented 6 years ago

@frederickjh Fish internally in complete.cpp has a special-case that executes our __fish_describe_command script for external commands.

This script uses the system tool apropos which queries the machine's whatis database (which is cached). The script just massages the output using awk.

One could imagine a dirty change to complete.cpp that made it always use an augmented version of that fish script to describe functions. It could be responsible for the caching. But it gives me a kind of sick feeling.

I'm surprised we implemented that in fish script at all (reminds me of our thing that prints help for commands), rather than figure out how to read the whatis database directly. Maybe implementations vary platform-to-platform.

frederickjh commented 6 years ago

OK. That bring to mind the question of is there a way that we could have the fish shell function descriptions added to the whatis database? Not sure if that is the best solution, just an idea.

frederickjh commented 4 years ago

I have reworked the code in this comment and have created a function that will load the functions descriptions, however I cannot get this to work in a configuration file at Fish shell start. So, you will need to run this function after opening a new terminal.

~/.config/fish/functions/load-em.fish

function load-em --description 'Loads Fish shell function descriptions.'
# Load function information so it shows up in auto completion
# Original from https://github.com/fish-shell/fish-shell/issues/1915#issuecomment-72315918

  for i in (functions | tr , ' ')
    functions $i > /dev/null
  end
end
goude commented 3 years ago

@frederickjh thanks for this, I was able to get it to run from config.fish after renaming the function to load_em.

I have reworked the code in this comment and have created a function that will load the functions descriptions, however I cannot get this to work in a configuration file at Fish shell start. So, you will need to run this function after opening a new terminal.

benkeil commented 2 years ago

any progress after 10 years? 😂

dfpetrin commented 2 years ago

I have reworked the code in this comment and have created a function that will load the functions descriptions, however I cannot get this to work in a configuration file at Fish shell start. So, you will need to run this function after opening a new terminal.

~/.config/fish/functions/load-em.fish

function load-em --description 'Loads Fish shell function descriptions.'
# Load function information so it shows up in auto completion
# Original from https://github.com/fish-shell/fish-shell/issues/1915#issuecomment-72315918

  for i in (functions | tr , ' ')
    functions $i > /dev/null
  end
end

@frederickjh thanks for this, I was able to get it to run from config.fish after renaming the function to load_em.

I have reworked the code in this comment and have created a function that will load the functions descriptions, however I cannot get this to work in a configuration file at Fish shell start. So, you will need to run this function after opening a new terminal.

Thanks @frederickjh and @goude, this works like a charm 🤓

floam commented 2 years ago

I have an approach in mind that will obviate some of these issues, but only for functions we ship.

andradei commented 1 month ago

This makes functions defined anywhere other than $fish_function_path preferable from a UX standpoint. The performance hit isn't even that big if you have a bunch of tiny functions, and multiple functions can be defined per file.