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

Menu redesign #49

Closed
wants to merge 7 commits into from
Closed

Menu redesign #49

wants to merge 7 commits into from

Conversation

naveci
Copy link
Contributor

@naveci naveci commented Feb 10, 2020

Description

Reworked the menus in Sooty

Does this fix a known existing bug under Issues?

Not a bug, but referenced under #39.

Type of Change

Please delete any options that do not apply here:

  • Bug Fix
  • New Feature
  • Requires documentation additions / changes
  • Other, Code improvement

Any further info related to the addition

This needs to properly tested before merging with master. Also, any changes during the time of the dev branch creation and merge with master need to be reviewed.
I think I got all the nasty things out, but would prefer to have someone else taking a look at it.

Example: I had to add a prompt for enter function, otherwise the result would immediately be cleared from the screen and the user would be presented with the menu again.

@naveci
Copy link
Contributor Author

naveci commented Feb 10, 2020

i see I'm already a bit behind. That's fine, most of the merge will just replace the menu functions.
The only thing that has to be reviewed is the enter to continue prompts.

Perhaps it's a good thing to leave a bit of code in Sooty and split out the rest to the modules.
For example, for Phishtank, there is code in Sooty.py to handle the config and input parts, then pass that data on to the separate module: ab48e80

If this would be the case for all modules, then it would be possible to put the enter-to-continue function in Sooty.py (it's tied to the console-menu module!). Yet, you can still develop the module as stand-alone which maked it easier for testing.
Thoughts?

@TheresAFewConors
Copy link
Owner

aged out. Reopen if still relevant

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.

2 participants