-
Notifications
You must be signed in to change notification settings - Fork 35
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
Add completion for fishshell #440
Add completion for fishshell #440
Conversation
953ae4d
to
50e2f59
Compare
I'm unsure what to do with the spellcheck errors reported by the CI |
I think if you just add 'cp' and 'mkdir' to .github/wordlist.txt that will take care of it. |
50e2f59
to
b1fe089
Compare
I'm unsure if we should update "debian/rules" or create "debian/install" to ship the completion and install them in /usr/share/fish/ and its subfolders |
Maybe something around meson also Like in this project https://github.com/swaywm/swaylock/blob/master/completions%2Fmeson.build |
Yes, good idea.
The Debian rules file executes meson install and meson test (even though it's not obvious) so if an install target is added for the fish file it will get included with the deb package automatically. |
@ccoVeille You don't need to squash your commits, but you certainly may if you like to. I know some projects on GitHub require it, but I don't. |
I hesitated as it's always unclear how to behave on each project. Noted, I will take this into account and may split my PR in multiple commits |
b1fe089
to
64662e4
Compare
I added fish completion to meson by doing some reverse engineering 😁 Let me know if t's ok because I don't know what I'm doing 🤣 |
43c6ffd
to
c8f7e25
Compare
Other completions may be added later by other contributors. Fixes theimpossibleastronaut#436
c8f7e25
to
9457f93
Compare
I think we are ok now, what do you think ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ccoVeille You did very well with the meson build for having no experience with meson.
You can test changes like these by using meson install --destdir inst_test
in the build directory. That will create a directory called 'inst_test' and install everything there. You can navigate through the subdirectories to see where everything is going to. Then you can simply rm or rmw the inst_test directory when done.
You did great! Let me know if you have any questions about making the new changes I requested, or questions about what the code does. |
Getting closer. :) |
I'll make the suggested changes tomorrow or later. See you |
9457f93
to
e58632b
Compare
everything should be all set |
You forgot github words whitelist |
Thanks! I apologize for the delay, I haven't been able to spend much time on the computer lately. We'll have this merged soon though. I'm not entirely happy with it yet. I think I might just install all the completions into the rmw docdir instead of creating directories for each one in the datadir. I need to give it a little more thought. |
No problem. I'm not waiting for the feature. I already have it locally. That's why I contributed. |
A small reminder. This PR is still pending |
Copy completions to docdir
I think we are good |
Other completions may be added later by other contributors.
Fixes #436