On Fri Mar 8 21:54:34 2024 +0000, Henri Verbeet wrote:
```diff - if ((option = getopt_long(argc, argv, "htV", long_options,
NULL)) == -1)
+ if ((option = getopt_long(argc, argv, "ehtVo:", long_options, NULL)) == -1) ``` "o:" belongs in the next commit. ```diff + case 'e': + case OPTION_EMIT: + action_push(options, ACTION_TYPE_EMIT); + if (isatty(fileno(stdout))) + { + fprintf(stderr, "Output is a tty and output format is binary, exiting.\n" + "If this is really what you intended, specify the output explicitly.\n"); + return false; + } + break; ``` Relatedly, we can't specify the output explicitly yet in this commit. ```diff + "Currently this tool can only re-emit the same DXBC blob it loaded. However it\n" + "freshly recompute the checksum while doing so, so --emit can be useful\n" + "together with --ignore-checksum to fix the checksum for a blob.\n"; ``` I think the wording is a bit awkward here. I'd propose "However, it will recompute ..." or possibly "However, it recomputes ..." Note that multiple --emit options will cause vkd3d-dxbc to output the DXBC blob multiple times. I think that's fine, perhaps even desirable, but it may be worth explicitly pointing out. ```diff - if (!write_output(stdout, &serialized)) + if (!(output = open_output(options.output_filename, &close_output))) + { + vkd3d_shader_free_shader_code(&serialized); + goto done; + } + + if (!write_output(output, &serialized)) ``` That should use "action->u.output_filename" instead of "options.output_filename", right? Done, together with another couple of minor details I discovered in the meantime.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/582#note_64051