Skip to content

Add with options (#39)#55

Merged
ninoseki merged 8 commits intomainfrom
with-options
Jul 19, 2025
Merged

Add with options (#39)#55
ninoseki merged 8 commits intomainfrom
with-options

Conversation

@ninoseki
Copy link
Collaborator

Add with options to do #39.

To be honest, I'm not sure whether this is the cleanest solution for #39 but it works like the following:

# wait and download a DOM
urlscan scan submit <url> --with-dom
# wait and download a screenshot
urlscan submit <url> --with-screenshot
# wait and download both
urlscan submit <url> --with-both

@ninoseki ninoseki requested review from cdnsyseng, fw42 and heipei July 15, 2025 08:18
-t, --tags stringArray User-defined tags to annotate this scan
-v, --visibility string One of public, unlisted, private
-w, --wait Wait for the scan(s) to finish
--with-both Download both a screenshot and a DOM (this flag overrides wait, with-screen and with-both flags to true)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
--with-both Download both a screenshot and a DOM (this flag overrides wait, with-screen and with-both flags to true)
-d, --download Download screenshot and DOM contents (overrides wait/dom/screenshot)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed by c13f3a4

-v, --visibility string One of public, unlisted, private
-w, --wait Wait for the scan(s) to finish
--with-both Download both a screenshot and a DOM (this flag overrides wait, with-screen and with-both flags to true)
--with-dom Download a DOM (this flag overrides wait flag to true)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
--with-dom Download a DOM (this flag overrides wait flag to true)
--dom Download only the DOM contents (overrides wait)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same as the above (c13f3a4)

-w, --wait Wait for the scan(s) to finish
--with-both Download both a screenshot and a DOM (this flag overrides wait, with-screen and with-both flags to true)
--with-dom Download a DOM (this flag overrides wait flag to true)
--with-screenshot Download a screenshot (this flag overrides wait flag to true)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
--with-screenshot Download a screenshot (this flag overrides wait flag to true)
--screenshot Download only the screenshot (overrides wait)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same as the above (c13f3a4)

@cdnsyseng
Copy link
Contributor

cdnsyseng commented Jul 15, 2025

I think the flag names and descriptions could be improved. I didn't change other places in code in case we decide against the suggested changes.

IMHO, there is no need to specify --with- because including the flag implies the operation.

@ninoseki ninoseki requested a review from cdnsyseng July 15, 2025 08:46
@heipei
Copy link

heipei commented Jul 15, 2025

Didn't we agree in #39 that submit would just submit and return the UUID and that perform would block and wait and download the results?

@ninoseki
Copy link
Collaborator Author

@heipei

But anyway I will split the functionality into perform if you think that is cleaner than this implementation.

@ninoseki ninoseki requested a review from cdnsyseng July 15, 2025 09:27
@ninoseki ninoseki merged commit 77add93 into main Jul 19, 2025
5 checks passed
@ninoseki ninoseki deleted the with-options branch July 19, 2025 00:06
@ninoseki ninoseki mentioned this pull request Jul 20, 2025
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