Skip to content
This repository has been archived by the owner on Sep 7, 2023. It is now read-only.

fix: Remove english shell dependency in manage.sh script #1479

Merged
merged 4 commits into from
Apr 29, 2020

Conversation

milouse
Copy link
Contributor

@milouse milouse commented Jan 7, 2019

The current manage.sh script checks its internal methods using:

[ $(command -V "$ACTION" | grep ' function$')" = "" ]

A problem occurs when your shell locale is not english. Mine is french for example and command -V update_packages will return:

update_packages est une fonction
update_packages ()
{
    pip install --upgrade pip;
    pip install --upgrade setuptools;
    pip install -r "$BASE_DIR/requirements.txt"
}

fonction being different than function, the script does not work at all.

This pull requests replace the command by a sed one, which expect than the internal method will all use the same syntax. That way, method check is now language independant. However, the sed check is very naive for now and would need improvement the day more methods (or "private" methods) will be added to the script.

@milouse
Copy link
Contributor Author

milouse commented Jan 30, 2019

rebased on master

@dalf dalf added the core label Aug 2, 2019
@milouse
Copy link
Contributor Author

milouse commented Dec 15, 2019

Rebased on master

@codecov-io
Copy link

codecov-io commented Dec 15, 2019

Codecov Report

Merging #1479 into master will increase coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1479      +/-   ##
==========================================
+ Coverage   72.33%   72.38%   +0.04%     
==========================================
  Files         108      108              
  Lines        6210     6210              
  Branches     1072     1072              
==========================================
+ Hits         4492     4495       +3     
+ Misses       1441     1438       -3     
  Partials      277      277
Impacted Files Coverage Δ
searx/search.py 46.45% <0%> (+1.06%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3f93fe0...7b9c8f7. Read the comment docs.

@return42
Copy link
Contributor

Hi @milouse, please apologize the late response .. thanks for pointing out this bug! A more simpler solution might be:

LANG=C command -V ....

what do you think?

@return42 return42 added the bug label Dec 31, 2019
@milouse
Copy link
Contributor Author

milouse commented Jan 1, 2020

Effectivelly, it can be a more simple solution. I discover only recently the locale switch trick, that's why I didn't choose it initially. Feel free to use it instead!

@return42 return42 mentioned this pull request Apr 1, 2020
Copy link
Contributor

@return42 return42 left a comment

Choose a reason for hiding this comment

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

Thanks! .. and sorry for the delay :)

@return42 return42 merged commit d27331c into searx:master Apr 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants