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

render multi-line doc properly #31

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

eZioPan
Copy link
Contributor

@eZioPan eZioPan commented Apr 30, 2024

@eZioPan eZioPan marked this pull request as ready for review April 30, 2024 10:21
@Dirbaio
Copy link
Member

Dirbaio commented Apr 30, 2024

I'm not sure if respecting \n in the input is going to lead to more readable rustdocs. The use of newlines varies wildly across SVDs:

If we convert \n to a <br/> always, we'll make the "linebreaks are just word wraps" case worse. I think those are more common than the case where newlines do make sense.

How about instead we "declare" that the description field in the YAMLs is markdown? In this case the YAML->Rustdoc conversion is correct as-is. There's no need to split it into multiple #[doc]s, rustdoc already parses \n inside it normally. For example #[doc = "foo\n\nbar"] generates two markdown paragraphs, one with foo and one with bar.

If we do this, we'd have to fix the existing YAMLs in stm32-data manually instead. In cases where we do want a paragraph break, change \n to \n\n. Cases where the \n is a word-wrap we can leave as-is.

Then perhaps we can improve the SVD->YAML conversion instead to "markdownify" the descriptions. I'm not sure if this is possible to do automatically if the SVDs themselves already do \n for word-wrap...

@eZioPan
Copy link
Contributor Author

eZioPan commented Apr 30, 2024

The word wrap and mix ones are actually comes from me "just copy the description from RM manually". The line warp comes from PDF.

I have the same guessing on the totally lost newline in description.

For "we'll make the 'linebreaks are just word wraps' case worse.", at least VSCode doesn't do auto-wrap, it just put description in long one line. So I guess a "word wrap as newline" are ok?

For "'declare' that the description field in the YAMLs is markdown" and write "\n\n as a newline", we would introduce a new rule to YAML (description need to be a valid markdown), I don't know if future contributors will know this new rule easily, or follow it strictly.

And "\n\n" makes every line wrap with <p></p> which add extra margin between each line.

image

@eZioPan
Copy link
Contributor Author

eZioPan commented May 1, 2024

I change my code, that it use only one #[doc=] to render multiline doc

#[doc = "Scaling factor.  \nInput value has been multiplied by 2^(-n) before for argument.  \nOutput value will need to be multiplied by 2^n later for results."]

And, omit any newline in description seems intentional?

chiptool/src/main.rs

Lines 194 to 197 in 1c198ae

// Fix weird newline spam in descriptions.
(Regex::new("[ \n]+").unwrap(), " "),
// Fix weird tab and cr spam in descriptions.
(Regex::new("[\r\t]+").unwrap(), " "),

Should we change the rules to

([ \n]){2,} map to $1, \r+ map to \n, and \t+ map to ?

But, if we make the change, then fix already generated yaml files could be a big project ...

@eZioPan
Copy link
Contributor Author

eZioPan commented May 7, 2024

I have change the description_cleanups part of main.rs. Here is a sample I generated:

CORDIC of stm32h562.svd, upstream output compare to my output

20c20,29
<     description: 'Function 2: Phase 3: Modulus 4: Arctangent 5: Hyperbolic cosine 6: Hyperbolic sine 7: Arctanh 8: Natural logarithm 9: Square Root.'
---
>     description: |-
>       Function
>       2: Phase
>       3: Modulus
>       4: Arctangent
>       5: Hyperbolic cosine
>       6: Hyperbolic sine
>       7: Arctanh
>       8: Natural logarithm
>       9: Square Root.
24,25c33,36
<     description: Precision required (number of iterations) To determine the number of iterations needed for a given accuracy refer to . Note that for most functions, the
< recommended range for this field is 3 to 6.
---
>     description: |-
>       Precision required (number of iterations)
>       To determine the number of iterations needed for a given accuracy refer to .
>       Note that for most functions, the recommended range for this field is 3 to 6.
29,30c40,42
<     description: Scaling factor The value of this field indicates the scaling factor applied to the arguments and/or results. A value n implies that the arguments have been multiplied by a factor 2-n, and/or the results need to be multiplied by 2n. Refer to for the applicability of the scaling factor for each function and the appropriate
< range.
---
>     description: |-
>       Scaling factor
>       The value of this field indicates the scaling factor applied to the arguments and/or results. A value n implies that the arguments have been multiplied by a factor 2-n, and/or the results need to be multiplied by 2n. Refer to for the applicability of the scaling factor for each function and the appropriate range.
34c46,48
<     description: Enable interrupt. This bit is set and cleared by software. A read returns the current state of the bit.
---
>     description: |-
>       Enable interrupt.
>       This bit is set and cleared by software. A read returns the current state of the bit.
38c52,54
<     description: Enable DMA read channel This bit is set and cleared by software. A read returns the current state of the bit.
---
>     description: |-
>       Enable DMA read channel
>       This bit is set and cleared by software. A read returns the current state of the bit.
42c58,60
<     description: Enable DMA write channel This bit is set and cleared by software. A read returns the current state of the bit.
---
>     description: |-
>       Enable DMA write channel
>       This bit is set and cleared by software. A read returns the current state of the bit.
46c64,66
<     description: Number of results in the CORDIC_RDATA register Reads return the current state of the bit.
---
>     description: |-
>       Number of results in the CORDIC_RDATA register
>       Reads return the current state of the bit.
50c70,72
<     description: Number of arguments expected by the CORDIC_WDATA register Reads return the current state of the bit.
---
>     description: |-
>       Number of arguments expected by the CORDIC_WDATA register
>       Reads return the current state of the bit.
54c76,80
<     description: Width of output data RESSIZE selects the number of bits used to represent output data. If 32-bit data is selected, the CORDIC_RDATA register contains results in q1.31 format. If 16-bit data is selected, the least significant half-word of CORDIC_RDATA contains the primary result (RES1) in q1.15 format, and the most significant half-word contains the secondary result (RES2), also in q1.15 format.
---
>     description: |-
>       Width of output data
>       RESSIZE selects the number of bits used to represent output data.
>       If 32-bit data is selected, the CORDIC_RDATA register contains results in q1.31 format.
>       If 16-bit data is selected, the least significant half-word of CORDIC_RDATA contains the primary result (RES1) in q1.15 format, and the most significant half-word contains the secondary result (RES2), also in q1.15 format.
58c84,88
<     description: Width of input data ARGSIZE selects the number of bits used to represent input data. If 32-bit data is selected, the CORDIC_WDATA register expects arguments in q1.31 format. If 16-bit data is selected, the CORDIC_WDATA register expects arguments in q1.15 format. The primary argument (ARG1) is written to the least significant half-word, and the secondary argument (ARG2) to the most significant half-word.
---
>     description: |-
>       Width of input data
>       ARGSIZE selects the number of bits used to represent input data.
>       If 32-bit data is selected, the CORDIC_WDATA register expects arguments in q1.31 format.
>       If 16-bit data is selected, the CORDIC_WDATA register expects arguments in q1.15 format. The primary argument (ARG1) is written to the least significant half-word, and the secondary argument (ARG2) to the most significant half-word.
62,63c92,95
<     description: Result ready flag This bit is set by hardware when a CORDIC operation completes. It is reset by hardware when the CORDIC_RDATA register is read (NRES+1)
< times. When this bit is set, if the IEN bit is also set, the CORDIC interrupt is asserted. If the DMAREN bit is set, a DMA read channel request is generated. While this bit is set, no new calculation is started.
---
>     description: |-
>       Result ready flag
>       This bit is set by hardware when a CORDIC operation completes. It is reset by hardware when the CORDIC_RDATA register is read (NRES+1) times.
>       When this bit is set, if the IEN bit is also set, the CORDIC interrupt is asserted. If the DMAREN bit is set, a DMA read channel request is generated. While this bit is set, no new calculation is started.
70c102,107
<     description: Function result If 32-bit format is selected (CORDIC_CSR.RESSIZE = 0) and two output values are expected (CORDIC_CSR.NRES = 1), this register must be read twice when the RRDY flag is set. The first read fetches the primary result (RES1). The second read fetches the secondary result (RES2) and resets RRDY. If 32-bit format is selected and only one output value is expected (NRES = 0), only one read of this register is required to fetch the primary result (RES1) and reset the RRDY flag. If 16-bit format is selected (CORDIC_CSR.RESSIZE = 1), this register contains the primary result (RES1) in the lower half, RES[15:0], and the secondary result (RES2) in the upper half, RES[31:16]. In this case, NRES must be set to 0, and only one read performed. A read from this register resets the RRDY flag in the CORDIC_CSR register.
---
>     description: |-
>       Function result
>       If 32-bit format is selected (CORDIC_CSR.RESSIZE = 0) and two output values are expected (CORDIC_CSR.NRES = 1), this register must be read twice when the RRDY flag is set. The first read fetches the primary result (RES1). The second read fetches the secondary result (RES2) and resets RRDY.
>       If 32-bit format is selected and only one output value is expected (NRES = 0), only one read of this register is required to fetch the primary result (RES1) and reset the RRDY flag.
>       If 16-bit format is selected (CORDIC_CSR.RESSIZE = 1), this register contains the primary result (RES1) in the lower half, RES[15:0], and the secondary result (RES2) in the upper half, RES[31:16]. In this case, NRES must be set to 0, and only one read performed.
>       A read from this register resets the RRDY flag in the CORDIC_CSR register.
77,79c114,121
<     description: Function input arguments This register is programmed with the input arguments for the function selected in the CORDIC_CSR register FUNC field. If 32-bit
< format is selected (CORDIC_CSR.ARGSIZE = 0) and two input arguments are required (CORDIC_CSR.NARGS = 1), two successive writes are required to this register. The first writes the primary argument (ARG1), the second writes the secondary argument (ARG2). If 32-bit format is selected and only one input argument is required (NARGS = 0), only
< one write is required to this register, containing the primary argument (ARG1). If 16-bit format is selected (CORDIC_CSR.ARGSIZE = 1), one write to this register contains both arguments. The primary argument (ARG1) is in the lower half, ARG[15:0], and the secondary argument (ARG2) is in the upper half, ARG[31:16]. In this case, NARGS must be set to 0. Refer to for the arguments required by each function, and their permitted range. When the required number of arguments has been written, the CORDIC evaluates the function designated by CORDIC_CSR.FUNC using the supplied input arguments, provided any previous calculation has completed. If a calculation is ongoing, the ARG1 and ARG 2 values are held pending until the calculation is completed and the results read. During this time, a write to the register cancels the pending operation and overwrite the argument data.
---
>     description: |-
>       Function input arguments
>       This register is programmed with the input arguments for the function selected in the CORDIC_CSR register FUNC field.
>       If 32-bit format is selected (CORDIC_CSR.ARGSIZE = 0) and two input arguments are required (CORDIC_CSR.NARGS = 1), two successive writes are required to this register. The first writes the primary argument (ARG1), the second writes the secondary argument (ARG2).
>       If 32-bit format is selected and only one input argument is required (NARGS = 0), only one write is required to this register, containing the primary argument (ARG1).
>       If 16-bit format is selected (CORDIC_CSR.ARGSIZE = 1), one write to this register contains both arguments. The primary argument (ARG1) is in the lower half, ARG[15:0], and the secondary argument (ARG2) is in the upper half, ARG[31:16]. In this case, NARGS must be set to 0.
>       Refer to for the arguments required by each function, and their permitted range.
>       When the required number of arguments has been written, the CORDIC evaluates the function designated by CORDIC_CSR.FUNC using the supplied input arguments, provided any previous calculation has completed. If a calculation is ongoing, the ARG1 and ARG 2 values are held pending until the calculation is completed and the results read. During this time, a write to the register cancels the pending operation and overwrite the argument data.

I still think use hard-linebreak in yaml file is better than write a bunch of \n\n, it better for editors to read and modify (the yaml file). For this reason, I still perfer "markdownify" description on rendering it to rust doc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants