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

Process multiple files and other tweaks #9

Closed
wants to merge 2 commits into from
Closed

Process multiple files and other tweaks #9

wants to merge 2 commits into from

Conversation

danishalim
Copy link
Contributor

Enabled processing multiple files, made code more modular, used try..catch blocks and used best practices code e.g. removed "process.exit" and replaced with "throw."

Copy link
Owner

@hyunjiLeeTech hyunjiLeeTech left a comment

Choose a reason for hiding this comment

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

Thank you for your pull request. I've found some issues on your change.
When I process my tool using any argument, it doesn't work.
For example, if I enter 'url-fi -s test.html', it gives me an error.
My previous code allows me to enter multiple arguments at the same time so please check this. (e.g. url-fi -shv test.html)
Thanks.

@danishalim
Copy link
Contributor Author

Hi, I have fixed the options parsing issue. All options work now including the "s" option.

Copy link
Owner

@hyunjiLeeTech hyunjiLeeTech left a comment

Choose a reason for hiding this comment

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

Can you please check it again? When I enter 'url-fi -sh test.html', it only displays the help messages without working -s option. Thank you.

@danishalim
Copy link
Contributor Author

That is odd. It works fine on my end. https://imgur.com/a/Z2ebRtf. Did you test this PR?

Copy link
Owner

@hyunjiLeeTech hyunjiLeeTech left a comment

Choose a reason for hiding this comment

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

Hi, I've checked your code and see it works at some point. However, it has one point missed from my expectation.
In my tool, I would like to put argument located in any places (e.g., 'url-fi -hvs test.html' and 'url-fi test.html -hvs' display the same result. However, when I look at your code, it seems that if the first one in a loop does not include '-', it goes to check url function. Let me know if I did not figure out your code. Thanks.

@hyunjiLeeTech hyunjiLeeTech added bug Something isn't working and removed bug Something isn't working labels Nov 12, 2020
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