Skip to content

feat: major refactoring and breaking changes - #145

Merged
Ullaakut merged 6 commits into
masterfrom
v4
Jan 31, 2026
Merged

feat: major refactoring and breaking changes#145
Ullaakut merged 6 commits into
masterfrom
v4

Conversation

@Ullaakut

@Ullaakut Ullaakut commented Jan 27, 2026

Copy link
Copy Markdown
Owner

Goal of this PR

Refactors the library into a v4 API with clearer sync/async execution paths, improved context propagation, progress streaming, and expanded examples/tests (including Docker-based compatibility tests).

Changes

  • Introduces new Run(ctx) / RunAsync(ctx) APIs and moves warnings onto the Run result (Run.Warnings()).
  • Adds separated sync/async/progress execution implementations and shared helpers.
  • Updates tests, examples, tooling (Go version, golangci-lint, GitHub workflows), and documentation for v4.

How to test it

  • Read the changes, read the docs.
  • Make sure you have nmap and docker installed.

Then,

  • make lint
  • make fmt
  • make test

Notes

Fixes #128

Closes #31 — Implementation is now completely different.
Closes #113 — Seemingly already fixed, but need to confirm with involved parties that default value is OK.

@Ullaakut
Ullaakut requested review from Copilot and elivlo January 27, 2026 14:02
@Ullaakut Ullaakut self-assigned this Jan 27, 2026
@Ullaakut Ullaakut added the enhancement New feature or request label Jan 27, 2026

This comment was marked as outdated.

* Fixes warnings being overwritten in processResult
* Fixes finalizeRun incorrectly returning
* Update README async example
* Improve multiple examples
* Remove unused error sentinel
* Call os.Exit in TestMain
@Ullaakut
Ullaakut marked this pull request as ready for review January 28, 2026 20:32
@Ullaakut

Copy link
Copy Markdown
Owner Author

PSA

This branch despite not being merged, is already used by the latest (pre-release) v6.0.0 of Cameradar.

I will consider this PR a blocker for Cameradar to get out of pre-release and get an official v6 release. I am hoping to get some human reviews on this PR, but if by the end of February 2026 it does not, I will give it a few double checks myself and go ahead :)

@TerminalFi

Copy link
Copy Markdown
Collaborator

Seems good to me. Working in my local app I use it for and loving the simplicity.

@TerminalFi TerminalFi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Generally speaking good stuff! Could simplify 60% or so of the appends since append accepts multiple args / slice. This would just shorten code but not much else.

Comment thread opt_firewall.go Outdated
Comment thread opt_firewall.go Outdated
@Ullaakut

Copy link
Copy Markdown
Owner Author

@TerminalFi Fixed all occurrences of unnecessary double appends, and actually while doing so I realized the --scanflags option was broken in two fun ways 😄

  • It was using a hex-formatted value for the scan flags, but not preceding it with 0x so it was considered as if it was decimal, making any combination of flags beyond 9 wrong
  • If anyone tried using it and turning on all flags at once, we'd run into a 23 year old nmap bug where the flags are still expected to be within one bit even though in RFC 3540 (2003) the flags were extended, nmap supports those flags, but does not support a decimal value beyond 255 😁

So now I switched it to using string values, since those are not susceptible to this bug. They make the readability of the args arguably worse when lots of flags are provided/better when less are provided, but anyway there's no way around it without opening a PR against nmap itself.

Thanks so much for your review 🙏

@Ullaakut
Ullaakut merged commit 5b5552b into master Jan 31, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Example should be updated Run nmap without runtime interaction Review Async implementation vs Channels Implementation.

3 participants