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

use fully qualified paths of cp, mv, etc. or command before invoking them #79

Closed
jab opened this issue Feb 7, 2016 · 12 comments · Fixed by #80
Closed

use fully qualified paths of cp, mv, etc. or command before invoking them #79

jab opened this issue Feb 7, 2016 · 12 comments · Fixed by #80
Milestone

Comments

@jab
Copy link

jab commented Feb 7, 2016

I have common commands like cp, mv, etc. aliased to cp -i, mv -i, etc.

When I use fisherman, I see overwrite /Users/jab/.config/fisherman/fishfile? (y/n [n]) not overwritten.

I assume this is caused by fisherman referring to a command like cp as just cp, rather than as /bin/cp or command cp. This can cause problems if the user has the command aliased to pass other arguments (such as interactive mode, as above), since fisherman is assuming cp is the same thing as /bin/cp, it isn't. If so, any interest in fixing this? I think well-behaved shell scripts are supposed to use fully-qualified paths or use command to avoid this problem.

@daenney
Copy link
Contributor

daenney commented Feb 7, 2016

Using command is probably a good idea but if you're using Fish I'd recommend using abbreviations over aliases. They have the advantage of being automatically expanded on your shell (so the command you end up executing doesn't contain "hidden" options) and wouldn't mess with scripts like this.

@jab
Copy link
Author

jab commented Feb 7, 2016

Thanks @daenney, switched to using abbr and glad to have another way to accomplish the same thing that doesn't get in the way of fisherman. And glad you agree it's worth using command in fisherman.

@ghost ghost added the ENHANCEMENT label Feb 7, 2016
@ghost ghost added this to the 0.7.0 milestone Feb 7, 2016
@ghost ghost added the CRITICAL label Feb 7, 2016
@ghost
Copy link

ghost commented Feb 7, 2016

@jab You are absolutely right, so I have added it to the next milestone. I'll patch this ASAP and push after all tests pass and I see things to be working as they should.

@ghost
Copy link

ghost commented Feb 7, 2016

  • mkdir
  • cp
  • ln
  • rm

@daenney
Copy link
Contributor

daenney commented Feb 7, 2016

Bash seems to have an unalias so you can remove aliases. It might be worth seeing if we can recreate that somehow so we can call that at the top/whenever we start a script and then not have to worry about the command invocations at all.

@daenney
Copy link
Contributor

daenney commented Feb 7, 2016

Oh ha, since aliases are just functions we could functions -e instead.

@ghost
Copy link

ghost commented Feb 7, 2016

@daenney Yes, we would need to copy the function first, remove it and then define it again later.

@daenney
Copy link
Contributor

daenney commented Feb 7, 2016

Oh, bummer. I was hoping it could scope that to the script currently being executed.

@ghost
Copy link

ghost commented Feb 7, 2016

scope that to the process currently being executed.

That's clever, and I wish it was like that too. But that trick only works for variables in Fish.

function foo
    set -l x 1
    begin
        set -l x 
    end
    echo $x # 1
end

@daenney
Copy link
Contributor

daenney commented Feb 7, 2016

In theory though, functions has -c which would allow you to copy the function to a backup and back but it would affect all other running sessions too which is unpleasant.

@ghost
Copy link

ghost commented Feb 7, 2016

I think we need to command patch stuff like cp and rm first, to fix @jab's problem and probably the most common use case out there.

As for other commands like printf, git or make we should be able to spare them for the time being with little or no consequences.

@daenney
Copy link
Contributor

daenney commented Feb 7, 2016

I've updated it to call command on rm, ln, mkdir and mv.

ghost pushed a commit that referenced this issue Feb 11, 2016
+ Add  the ability  to install plugins  from Gists.  You can
distribute a very simple,  one-single function plugin in the
form  of a  Gist. Your  users  can install  it using  fisher
install  url and  Fisherman will  query the  Gist using  the
GitHub API to get a list of  the Gist files and use the name
of the  first identified *.fish  file to name the  plugin in
your system.  Since there is no  formal way to name  a Gist,
and you may  prefer to keep the "description"  field for the
actual description  and not a name,  Fisherman supports only
one fish file per Gist. Closes #75.

+ Use command(1) when calling non-builtins. Thanks @daenney.
Closes #79.

+  Add  __fisher_plugin_can_enable  to detect  installing  a
prompt that is not the current one. Closes #78.

+  Remove  the ability  to  install  a  plugin in  a  parent
directory using ..  or ../ or even worse, ../../  as well as
other combinations  that navigate  to a parent  directory. I
find  the use  case odd  at  best, and  more dangerous  that
useful.  If you  want  to  install a  local  plugin use  the
full  path  or a  relative  path,  always top  down.  fisher
install  . or  fisher  install my/plugin  or fisher  install
/Users/$USER/path/to/plugin. Closes #81.
@ghost ghost closed this as completed in #80 Feb 11, 2016
jorgebucaran pushed a commit that referenced this issue Sep 21, 2016
By using `command` we ensure that we always end up calling the external
binary and don't get caught by user aliasses, functions or other
magic.

Closes #79
jorgebucaran pushed a commit that referenced this issue Sep 21, 2016
+ Add  the ability  to install plugins  from Gists.  You can
distribute a very simple,  one-single function plugin in the
form  of a  Gist. Your  users  can install  it using  fisher
install  url and  Fisherman will  query the  Gist using  the
GitHub API to get a list of  the Gist files and use the name
of the  first identified *.fish  file to name the  plugin in
your system.  Since there is no  formal way to name  a Gist,
and you may  prefer to keep the "description"  field for the
actual description  and not a name,  Fisherman supports only
one fish file per Gist. Closes #75.

+ Use command(1) when calling non-builtins. Thanks @daenney.
Closes #79.

+  Add  __fisher_plugin_can_enable  to detect  installing  a
prompt that is not the current one. Closes #78.

+  Remove  the ability  to  install  a  plugin in  a  parent
directory using ..  or ../ or even worse, ../../  as well as
other combinations  that navigate  to a parent  directory. I
find  the use  case odd  at  best, and  more dangerous  that
useful.  If you  want  to  install a  local  plugin use  the
full  path  or a  relative  path,  always top  down.  fisher
install  . or  fisher  install my/plugin  or fisher  install
/Users/$USER/path/to/plugin. Closes #81.
jorgebucaran pushed a commit that referenced this issue Sep 7, 2018
By using `command` we ensure that we always end up calling the external
binary and don't get caught by user aliasses, functions or other
magic.

Closes #79
jorgebucaran added a commit that referenced this issue Sep 7, 2018
+ Add  the ability  to install plugins  from Gists.  You can
distribute a very simple,  one-single function plugin in the
form  of a  Gist. Your  users  can install  it using  fisher
install  url and  Fisherman will  query the  Gist using  the
GitHub API to get a list of  the Gist files and use the name
of the  first identified *.fish  file to name the  plugin in
your system.  Since there is no  formal way to name  a Gist,
and you may  prefer to keep the "description"  field for the
actual description  and not a name,  Fisherman supports only
one fish file per Gist. Closes #75.

+ Use command(1) when calling non-builtins. Thanks @daenney.
Closes #79.

+  Add  __fisher_plugin_can_enable  to detect  installing  a
prompt that is not the current one. Closes #78.

+  Remove  the ability  to  install  a  plugin in  a  parent
directory using ..  or ../ or even worse, ../../  as well as
other combinations  that navigate  to a parent  directory. I
find  the use  case odd  at  best, and  more dangerous  that
useful.  If you  want  to  install a  local  plugin use  the
full  path  or a  relative  path,  always top  down.  fisher
install  . or  fisher  install my/plugin  or fisher  install
/Users/$USER/path/to/plugin. Closes #81.
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants