Use escript directly to run ex_doc #8490
Conversation
CT Test Results 3 files 143 suites 49m 17s ⏱️ Results for commit cf52f5f. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts// Erlang/OTP Github Action Bot |
| @@ -7330,7 +7330,7 @@ fi | |||
| done | |||
|
|
|||
| if test -z "$EX_DOC"; then | |||
| EX_DOC="$ERL_TOP/make/ex_doc_wrapper" | |||
| EX_DOC="$ERL_TOP/make/ex_doc_wrapper.erl" | |||
There was a problem hiding this comment.
TBH I am unsure what the convention is for writing escript files. When I open an erl file inside this source directory I still get all my LSP niceties so I just used .erl
| ExDocExe -> | ||
| Cmd = lists:append(["exec ", ExDocExe, " "], lists:join(" ", Args)), | ||
| io:format("Running ~s~n", [Cmd]), | ||
| os:cmd(Cmd) |
There was a problem hiding this comment.
I tried the same
Out = os:cmd(Cmd),
io:format("~s", [Out])
here, but I got a weird error when running ex_doc so I didn't worry about it.
There was a problem hiding this comment.
I think it has to do with the output being colorized
| {error, _Description} -> | ||
| io:format("Unable to get line?~n"), | ||
| {error, try_again}; | ||
| eof -> | ||
| io:format("Got end of file?~n"), | ||
| {error, try_again}; |
There was a problem hiding this comment.
Not exactly sure what {error, _} would arise from but it seems safe to try again and let them Ctrl-C
I guess eof is just an empty line so we should try again.
| Input -> | ||
| case string:casefold(string:trim(Input)) of | ||
| "y" -> {ok, download}; | ||
| "n" -> {error, do_not_download}; |
There was a problem hiding this comment.
If they explicitly ask to not download ex_doc we just quit out
| case string:casefold(string:trim(Input)) of | ||
| "y" -> {ok, download}; | ||
| "n" -> {error, do_not_download}; | ||
| _ -> {error, try_again} |
There was a problem hiding this comment.
The didn't respond with y, Y, N or n so we try again.
| % Passing in --option "hello world" leads to ["--option", "hello world"] | ||
| % Re-wrap the latter in quotes |
There was a problem hiding this comment.
This happens in one spot as far as I can tell.
| end. | ||
|
|
||
| quote_arguments(String) -> | ||
| case string:find(String, " ") of |
There was a problem hiding this comment.
This matches Erlang System Documentation and pads it to "Erlang System Documentation" so the os:cmd doesn't fail.
|
Hello! I think that it would be better to do it the other way around, that is always use the bash wrapper and in that call escript. One reason is that we want the output from ex_doc to end up in the console so that we can view warnings and error messages. To do that I would create an ex_doc_wrapper.in file and then use configure to insert the correct |
Totally fair! I believe I can do this, but it is going to take me a bit of time. Trying to wrap up other OSS work. I appreciate your review! |
|
No worries, we are in no hurry. I will close this PR, looking forward to your new one :) |
Closes #8378
Relying on
#! /usr/bin/env escriptto runex_docdoesn't really work well in nixpkgs builds. We actually know the location ofescriptalready so we can just use$ERL_TOP/escriptto run ex_doc and we no longer have to rely on/usr/bin/envat all.One issue arises though, you can't run a bash script with
escript(i.e.make/ex_doc_wrapper) however we can run an equivalent erlang escript!Let me know what you think!