Skip to content

Fix pysearch command validation to only allow python scripts ( #30) #33

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

Merged
merged 21 commits into from
Jun 26, 2025

Conversation

pushkar-hue
Copy link

Fixed pysearch command validation to only allow python scripts. Before it used to accept every command entered now it checks them first and guide the user to use the QuickFlow's search feature instead. This PR will close the issue #30

SpaciousCoder78 and others added 18 commits June 8, 2025 21:15
feat: Added make for build system and updated docs
fix: fixed all build issues with ubuntu and arch linux
fix: removed strict python3 versioning requirement
@SpaciousCoder78
Copy link
Owner

Thanks for the commit.

You should’ve asked to get the issue assigned to you. In general it’s a good practice to only work on the issue that’s assigned to you.

Nevertheless I will test it when I get some time but again I think you may need to make some more changes as there will be a merge conflict with another contributor’s PR from yesterday.

I will let you know about the code review later this week.

@SpaciousCoder78 SpaciousCoder78 changed the base branch from main to development June 11, 2025 02:21
@pushkar-hue
Copy link
Author

No worries, take your time testing it. If there’s a merge conflict or anything else that needs fixing, just ping me and I’ll handle it.

@Lonelyguy123
Copy link

Lonelyguy123 commented Jun 12, 2025

I'll be doing the code review
on behalf of @SpaciousCoder78

@SpaciousCoder78
Copy link
Owner

Hello @pushkar-hue, sorry for the delay. I was a bit busy last week.

I've just tested your code and I found the following issues with it:

The code does throw an exception when something else is entered but upon testing it further, I noticed that it throws exceptions even when python or python3 is used as an argument.

Example:

QuickOverflow > search
py-search > python3 test.py
QuickOverflow Error: Only 'python' or 'python3' commands are allowed.
Use QuickOverflow’s search feature to browse directories instead.
QuickOverflow > search      
py-search > python3
QuickOverflow Error: Only 'python' or 'python3' commands are allowed.
Use QuickOverflow’s search feature to browse directories instead.

If you can fix this small bug, we can merge your PR into development branch for further testing as this is a very important feature.

@pushkar-hue
Copy link
Author

Hey @SpaciousCoder78 , thanks for testing the PR. You are absolutely right there was an issue with
if not cmd.startswith("python") or cmd.startswith("python3"):

now fixed it, it works as intended now can you check again and let me know if there's anything else you'd like me to adjust.

@pushkar-hue
Copy link
Author

Hello @SpaciousCoder78, after submitting the PR I realized there was an issue with how submit_error from pyoverflow3 was being used. I've added a try-except block to handle any errors and ensure it works as expected. Let me know if there's anything else you'd like me to update.

@SpaciousCoder78
Copy link
Owner

I will review your code soon.

Can you also document your changes in the code explaining what changes you made?

It makes it a lot easier to review the changes.

@pushkar-hue
Copy link
Author

Hi @SpaciousCoder78 apologies for late reply, would you prefer me to document the changes as comments directly in the code, or in a separate file ? Happy to do whichever works best for you.

@SpaciousCoder78
Copy link
Owner

SpaciousCoder78 commented Jun 22, 2025 via email

@pushkar-hue
Copy link
Author

Added code comments to explain the changes made. Let me know if there's anything else you'd like me to do.

@SpaciousCoder78
Copy link
Owner

I've done the code review and testing and found that everything works well and perfect.

I'm merging this PR. Thank you for your contribution, I will mention you in the README

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 this pull request may close these issues.

3 participants