Skip to content

Enable key=value long options in OPT - #23

Merged
pfusik merged 1 commit into
pfusik:masterfrom
epi:epi/longopt
Oct 13, 2026
Merged

pfusik merged 1 commit into
pfusik:masterfrom
epi:epi/longopt

Conversation

@epi

@epi epi commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Prerequisite for stuff like opt 'cpu=65c02' (#16) or opt 'format=sdx' (#17).

@pfusik pfusik left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

We don't have to provide long aliases to all the short options right away. The long options should serve future expansions not 1:1 mapping of short options.

The obvious ones are listing=on, listing=off, cpu=65c02, cpu=6502.

Comment thread source/xasm/package.d Outdated
Comment thread source/xasm/package.d Outdated
Comment thread source/xasm/package.d Outdated

void setNamedOption(string name, bool hasValue, string value) {
switch (name) {
case "FILL":

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

How about combining three short options into one long:

 opt o-     = opt 'output=off'
 opt o+h-f- = opt 'output=raw'
 opt o+h-f+ = opt 'output=rom'
 opt o+h+f- = opt 'output=ataridos' ; default
 opt o+h+f+ ; not useful, no long alias

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Looks good. I'd add a few restrictions:

  • setting output is an error after anything has been emitted to object,
  • setting output twice is an error,
  • setting o, h, or f after output has been set explicitly is an error.

Without output, the single-letter options o, h and f work as previously.

@epi epi Sep 24, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Maybe the above are too restrictive – one may want to get label table and/or listing without the object bytes, and opt 'output=spartadosx'; opt o- would be a quite self-descriptive way to spell it, even if not the most elegant.

Currently o- is allowed even mid-block, which I consider a bug because it generates broken binaries. Not sure if there's any code out there that depends on it.

I'd disallow opt o- completely after the first object byte is emitted, unless there is a good reason to keep it allowed between blocks.

@epi epi Sep 24, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It may also make sense to get o out of output (so the latter would only set h and f), and provide 'object=on|off' so that it can be set in one opt with output.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ignore the last two comments. I don't think there's a use case for o- after output=....

Comment thread source/xasm/package.d Outdated
Comment thread source/xasm/package.d Outdated
Comment thread source/xasm/package.d Outdated
@epi
epi requested a review from pfusik September 24, 2026 15:09
@epi epi mentioned this pull request Sep 24, 2026
@epi
epi force-pushed the epi/longopt branch 2 times, most recently from bf51875 to e0a9180 Compare September 28, 2026 11:28
Comment thread source/xasm/package.d Outdated
Comment thread source/xasm/package.d Outdated
Comment thread source/xasm/package.d Outdated
Comment thread xasm.1.asciidoc Outdated
Comment thread xasm.1.asciidoc Outdated
Comment thread xasm.1.asciidoc Outdated
@epi
epi requested a review from pfusik October 11, 2026 15:48

@pfusik pfusik left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

LGTM!
Image

@pfusik
pfusik merged commit 38660dc into pfusik:master Oct 13, 2026
4 checks passed
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