Add line numbers to st.code #6042
Conversation
35b7e46 to
8fc6738
Compare
snehankekre
left a comment
There was a problem hiding this comment.
Approving the docstring change 👍
sfc-gh-tszerszen
left a comment
There was a problem hiding this comment.
Overall LGTM, before merging first please fix extra characters if they are not needed and the tests 👍
|
@sfc-gh-tszerszen I've resolved the comments about extra characters and why they are needed. Not sure what's needed to make the tests pass. Eslint is complaining about the "unist" import in the code block frontend code . I thought merging the latest changes from |
|
@sfc-gh-skekre thank you for clarification 👍 When it comes to missing import if it's missing dependency we need to add it. Otherwise if the dependency is present, but for some reason not visible, then maybe just ignore it? @sfc-gh-kbregula |
|
|
sfc-gh-mnowotka
left a comment
There was a problem hiding this comment.
I see a problem with this code. In two different places, on front-end and back-end it checks for the existence of an arbitrary string: showLineNumbers. This makes it hard to maintain. If for some reason that name will be changed in one of these two places, this feature will stop working.
Since we are already using protobuf, I'd like us to add showLineNumbers as a new optional field to Markdown.proto because IMO this is exactly why we are using protobufs.
b31f8f5 to
911f998
Compare
14a567f to
9fe3093
Compare
proto/streamlit/proto/Element.proto
Outdated
| Heading heading = 47; | ||
|
|
||
| // Next ID: 48 | ||
| Code code = 49; |
There was a problem hiding this comment.
I think this should be 48, because that's the next available ID. Doing a check I don't see 48 being used. (Feel free to adjust the comment to be more descriptive).
8f57e2b to
68c2437
Compare
## Describe your changes In [this PR](#6042), we added a new `def code` function in `code.py` which replaces the one in `markdown.py`. Since `CodeMixin` comes first in the definition of `DeltaGenerator` before `MarkdownMixin`, the `CodeMixin` version of the function takes precedence due to python's MRO algorithm and we can safely remove this function. ## GitHub Issue Link (if applicable) ## Testing Plan Covered by existing tests. --- **Contribution License Agreement** By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.
📚 Context
Close: #5756
Please describe the project or issue background here
What kind of change does this PR introduce?
🧠 Description of Changes
Add bullet points summarizing your changes here
Revised:
Insert screenshot of your updated UI/code here
Current:
Insert screenshot of existing UI/code here
🧪 Testing Done
🌐 References
Does this depend on other work, documents, or tickets?
Contribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.