Skip to content
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

Add parameter to specify documentation width in the shell #8651

Merged
merged 2 commits into from
Sep 11, 2024

Conversation

dmitrivereshchagin
Copy link
Contributor

It may be hard to read the documentation rendered over the whole width of the screen. The kernel configuration parameter is added to override this default behavior.

Copy link
Contributor

github-actions bot commented Jul 9, 2024

CT Test Results

    3 files    154 suites   1h 34m 1s ⏱️
3 560 tests 3 269 ✅ 287 💤 4 ❌
4 064 runs  3 738 ✅ 322 💤 4 ❌

For more details on these failures, see this check.

Results for commit f8a6528.

♻️ 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

@jhogberg jhogberg added team:VM Assigned to OTP team VM stalled waiting for input by the Erlang/OTP team labels Jul 15, 2024
@garazdawi
Copy link
Contributor

Hello! Thanks for the PR and I agree that this would be nice to have. However, it needs a testcase in lib/stdlib/test/shell_docs_SUITE.erl and documentation in lib/stdlib/doc/stdlib_app.md before it can be included.

@garazdawi garazdawi added waiting waiting for changes/input from author and removed stalled waiting for input by the Erlang/OTP team labels Aug 13, 2024
@dmitrivereshchagin dmitrivereshchagin force-pushed the stdlib/shell-docs-columns branch from 7db2448 to c28b60e Compare August 26, 2024 11:30
@dmitrivereshchagin
Copy link
Contributor Author

Test case and documentation are added. Parameter description is added in kernel_app.md instead of stdlib_app.md. The new parameter is added to the kernel application along with existing shell_docs_ansi.

Test case and documentation are also added for shell_docs_ansi.

@garazdawi, please take a look again.

@garazdawi
Copy link
Contributor

Great! I realize now that I was very unclear in my comment. The configuration option should be moved to the stdlib application as that is where most of the shell configuration is located. We cannot move the things that are in kernel for backward compatability reasons, but we want all new options to be in stdlib.

It may be hard to read the documentation rendered over the whole width
of the screen.  The stdlib configuration parameter is added to override
this default behavior.
@dmitrivereshchagin dmitrivereshchagin force-pushed the stdlib/shell-docs-columns branch from c28b60e to f8a6528 Compare August 26, 2024 12:32
@dmitrivereshchagin
Copy link
Contributor Author

Got it! Moved the new parameter to stdlib. Please take a look again :)

@garazdawi
Copy link
Contributor

Maybe for Erlang/OTP 28 I'll have to do something about the location of these parameters... the current status is a mess...

Your changes look good. I'll put it into testing and see if anything unexpected pops up.

@garazdawi garazdawi added the testing currently being tested, tag is used by OTP internal CI label Aug 26, 2024
@garazdawi garazdawi merged commit 7ce0bde into erlang:master Sep 11, 2024
15 of 17 checks passed
@garazdawi
Copy link
Contributor

Thanks!

@garazdawi garazdawi added this to the OTP-28.0 milestone Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team:VM Assigned to OTP team VM testing currently being tested, tag is used by OTP internal CI waiting waiting for changes/input from author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants