Skip to content
This repository has been archived by the owner on Dec 28, 2021. It is now read-only.

Applies fixes for Doc Parser. #1098

Merged
merged 36 commits into from
Feb 11, 2021
Merged

Applies fixes for Doc Parser. #1098

merged 36 commits into from
Feb 11, 2021

Conversation

BinarySoftware
Copy link
Contributor

@BinarySoftware BinarySoftware commented Jan 8, 2021

Pull Request Description

This PR bumps scala parser version to include latest commit to parser from engine repository, which fixes documentation tags, adds new tags according to specs as well as adds some of unavailable character, so that every doc in stdlib parses.

Important Notes

This PR wont solve issue with indent in first line, it will be fixed after updating AST.

Checklist

Please include the following checklist in your PR:

  • The documentation has been updated if necessary.
  • All code conforms to the Rust style guide.
  • All code has automatic tests where possible.
  • All code has been profiled where possible.
  • All code has been manually tested in the IDE.
  • All code has been manually tested in the "debug/interface" scene.
  • All code has been manually tested by the PR owner against our test scenarios.
  • All code has been manually tested by at least one reviewer against our test scenarios.

src/rust/ide/view/src/documentation.rs Outdated Show resolved Hide resolved
@farmaazon farmaazon added this to the 04-01-2021 milestone Jan 11, 2021
Copy link
Collaborator

@farmaazon farmaazon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see issues in some docs:

  1. A dot in a new line.
    image
  2. One word became an example:
    image

Are they a bugs in a docstrings, or in doc parser?

Copy link
Member

@wdanilo wdanilo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

With this (lack of) description I completely do not understand. what this PR really does.

@sylwiabr sylwiabr modified the milestones: 04-01-2021, 18-01-2021 Jan 19, 2021
@BinarySoftware
Copy link
Contributor Author

I see issues in some docs:

  1. A dot in a new line.
    image
  2. One word became an example:
    image

Are they a bugs in a docstrings, or in doc parser?

Hey Adam, those are bugs that i simply cant fix right now, as easily as i've fixed the tags or unavailable chars.

@BinarySoftware
Copy link
Contributor Author

image

With this (lack of) description I completely do not understand. what this PR really does.

Updated.

Copy link
Member

@wdanilo wdanilo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maciek, I asked you on Discord to create missing issues and link them here as a reply to Adam, it is not done yet, so I'm requesting changes.

@BinarySoftware
Copy link
Contributor Author

BinarySoftware commented Jan 30, 2021

Maciek, I asked you on Discord to create missing issues and link them here as a reply to Adam, it is not done yet, so I'm requesting changes.

Sorry for not doing that, here it comes:

enso-org/enso#1441

@farmaazon farmaazon modified the milestones: 18-01-2021, 1-02-2021 Feb 1, 2021
Base automatically changed from main to develop February 2, 2021 05:08
src/rust/ide/view/src/documentation.rs Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@farmaazon farmaazon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see still one not-working case in the workaround provided by this PR:

## A function foo doing foo like this:
       foo

Here the doc writer meant the second line to be intended, but we align the first line to the second.

I could accept this PR, as it is not a regression, but first confirm with @wdanilo that fixing that properly in the parser is impossible or too costly.

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Comment on lines 155 to 156
/// Fixes indentation in the first line of documentation by looking at next line, as documentation
/// doesn't know what was its indentation in the Enso source file.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please extend this description. What does "looking at next line" mean? Do we adjust the first line to the second? What if there is only one line? What if documentation author meant the second line to be indented?

If we really need to put a workaround here (not in parser), then I'd rather put three spaces to the first line (an equivalent of ## . Or maybe shall we put two spaces for ## instead? I'm not sure, I don't know how parser works).

Copy link
Contributor Author

@BinarySoftware BinarySoftware Feb 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, putting fixed amount of spaces wont fix the issue.
Take a look at some enso code:

type foo
    type bar
        ## docs here
           docs there
        type baz

We need to add the indentation of whole block plus the indent of 2 hashes - that is in this case 10 additional spaces, otherwise doc parser will get code like this:

 docs here
          docs there

and will create a synopsis with raw text "docs here" and code block of "docs there".

As we talked on dm, if there is only one line, there is no problem, it just wont do anything, and will work. If the next line is empty, there is still no problem, as what is in the next box will just become new paragraph, just as intended, in "body" section.

The only corner case is when the user specifically intends to make second line a code block, but that must be a really rare case, as i havent seen such in any doc in stdlib.

To sum up, this thing is really not a trivial fix as the doc code we get doesnt know anything about its origin, what was its indentation etc, hence the need to write such a hack. To be further discussed with @wdanilo but I dont see any other reasonable solution currently.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After talk I've removed hack method, this PR wont fix indent in first line.

src/rust/ide/view/src/documentation.rs Outdated Show resolved Hide resolved
src/rust/ide/view/src/documentation.rs Outdated Show resolved Hide resolved
src/rust/ide/view/src/documentation.rs Outdated Show resolved Hide resolved
@BinarySoftware BinarySoftware marked this pull request as draft February 8, 2021 22:21
@BinarySoftware BinarySoftware marked this pull request as ready for review February 9, 2021 07:19
Copy link
Collaborator

@farmaazon farmaazon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix a changelog entry before merge. I would rather not mention that our fixes are "hot".

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated
@@ -16,6 +16,7 @@ read the notes of the `Enso 2.0.0-alpha.1` release.
now on all supported platforms.
- [You can now see data frames in the table visualisation][1181]. Big tables get truncated to 2000
entries.
- [Fixing documentation layout in Searcher][1098]. Includes misinterpreted tags and unknown characters that are commonly used.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the changelog entries should be understandable to the end-users. Not all are perfect now, but this description "fixing documentation layout in Searcher" is not very understandable. Also, "fixing" means that it is being done, not done already, so its not a good form for a changelog. Can we describe here what was fixed and what kind of docs will be displayed better now ?

build/run.js Outdated Show resolved Hide resolved
@BinarySoftware BinarySoftware merged commit 4f82dcf into develop Feb 11, 2021
@BinarySoftware BinarySoftware deleted the wip/mm/doc-parser-bump branch February 11, 2021 16:00
mwu-tow pushed a commit to enso-org/enso that referenced this pull request Oct 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants