-
Notifications
You must be signed in to change notification settings - Fork 172
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
Deprecation of Options destinationDir #941
Comments
Interesting. We definitely need to do some investigation on this. asciidoctorj/asciidoctorj-core/src/main/java/org/asciidoctor/jruby/cli/AsciidoctorCliOptions.java Lines 321 to 322 in 7456234
But I can imagine that calling Options.setDestinationDir() could be completely useless, just looking at the code it seems like Asciidoctor would never look at this option when set from the API. |
It seems to me destinationDir is just a front-end for to_dir. I suspect for Python compatiblity https://github.com/asciidoc/asciidoc-py3/search?q=destination%3A&unscoped_q=destination%3A. While there's a mention of destinationDir and destination_dir in the code, there's no to_dir. |
The option only is used for the CLI, in that case the value is redirected to 'to_dir'. Fixes asciidoctor#853 Fixes asciidoctor#941
The option only is used for the CLI, in that case the value is redirected to 'to_dir'. Fixes asciidoctor#853 Fixes asciidoctor#941
Fix option to set ToDir internally. Deprecate destinationDir in Options and OptionsBuilder. Add JavaDoc pointing users to use toDir. Fixes asciidoctor#853 Fixes asciidoctor#941
I am confused about the use of
destinationDir
vstoDir
, I suspect they are the same and maybe the first one could be marked as deprecated to avoid confusion to users.Seeing this code
https://github.com/asciidoctor/asciidoctor/blob/1d61da9ee0aba63711c415fa629a80f275036aa4/lib/asciidoctor/cli/invoker.rb#L59 and the user manual https://asciidoctor.org/docs/user-manual/#ruby-api-options, seems pretty obvious but I wanted to contrast and maybe check if this has some history reason.
The text was updated successfully, but these errors were encountered: