FPGA-Research-Manchester / FABulous

Fabric generator and CAD tools
https://fabulous.readthedocs.io/en/latest/
Apache License 2.0
148 stars 34 forks source link

Fix typos, grammar, spelling and factual mistakes in the documentation #203

Closed IAmMarcelJung closed 3 months ago

IAmMarcelJung commented 4 months ago

After stumbling over another small mistake in the docmentation, I decided to check the whole documentation using ltex. As the name implies, it is normally used for LATEX documents, but it can also be used for reStructuredText (and others). I did of course check the suggestions and only applied the ones that made sense to me. There is of course also no guarantee that every error was catched, but I am quite happy with what it found.

In simulation.rst there also was a factual error, stating that 0xFAB0 causes sampling the data and creating a one-cycle strobe, although actually 0xFAB1 causes it.

As always, any comments or suggestions are appreciated!

IAmMarcelJung commented 4 months ago

I found even more mistakes and will include them in another commit.

IAmMarcelJung commented 4 months ago

Although the generation runs without errors, the generated files seem to be broken. I will investigate it and remove the draft if it is solved.

IAmMarcelJung commented 4 months ago

I messed up my local sphinx install. After repairing it, I could generate valid HTML files again and view the results. The changes can now be reviewed.

KelvinChung2000 commented 4 months ago

I will try to review them tomorrow.

Since you are working on the doc, it would be nice if you reviewed #197, which greatly enhances its appearance.

Probably not within this pull request, but it would also be nice if you could copy some of the yosys and nextpnr doc to FABulous doc to make FABulous more approachable for people not familiar with the Linux like #205

IAmMarcelJung commented 4 months ago

Thanks! I am not actively working on the documentation, this was mainly done for procastination...

However, I can still try improve the documentation and make it more beginner friendly. To my shame, I think I also did not document #138, so this also has to be done.

When I worked with the documentation, I noticed that the files are not formatted at all and some files had exceptionally long line, which made it a bit tedious to review them. Since we now use black as a formatter for python code, I looked for a restucturedText formatter. I only found rstfmt and docstrfmt. They both seem to be more or less active and the latter is influenced by the former. Should I open a discussion about if and which formatter to use?

KelvinChung2000 commented 4 months ago

I also should be working on something other than FABulous at the moment... procrastination...

More formatting would be nice. Since you are on the boat now, you can make the decision. Better doc support is a continuous effort, so do them when you don't want to work 😉. To be honest, once we moved to pip setup, things should become simpler. If the setup tool can also install yosys and nextpnr, then we don't even need to make doc for them.

IAmMarcelJung commented 4 months ago

Okay then I don't think it is necessary to document the installation of yosys and nextpnr any further.

I forgot to mention that I tried out docstrfmt, but it broke my HTML documents, although they were generated without any error. To get the generation working, I also had to change the sphinx version, so maybe it will also be fixed once #197 is done. I will definietly wait with the formatting until #197 and this PR are merged, since it might get confusing otherwise.

Good to know I'm not the only one who is procrastinating 😄

IAmMarcelJung commented 4 months ago

The last commit (d7cf183) implements using the todo extension. Now no TODO is in the generated docs anymore.

Also two TODOs are finished:

  1. Cite the "Shrink my FPGAs" paper where it was appropriate (I asked Dirk for this)
  2. Add a description for SHARED_PORT. An additional example of a SHARED_PORT was added in both the Verilog and the VHDL examples (Dirk told me to do so and I also think it is useful).

Two "bug fixes: were made:

  1. Changed Out_Pad to eFPGA for at the end of the eFPGA entity.
  2. Add end entity LUT4 in the example in the Bitstream remapping chapter.

I also asked Dirk for the other TODOs, but I'm still waiting for the answer.

If anyone else knows what has to be written for the TODOs you can of course also let me know or directly change it (I think only short additions are needed. Thanks in advance!

IAmMarcelJung commented 4 months ago

In the latest commits, another TODO could be fixed (thanks to @EverythingElseWasAlreadyTaken!). Also a small formulation and punctuation fix was done.

IAmMarcelJung commented 3 months ago

In the last commit (0b7f590) just the hint on the emulation setup was updated and turned into a note, so it does not look anymore as if it was just some internal comment.

IAmMarcelJung commented 3 months ago

Will it also lead to conflicts if I revert the changes made by black? Other than that I just added the todo extension. I could just revert the formatting if that is enough to avoid conflicts.

KelvinChung2000 commented 3 months ago

I thought this is going to development, never mind.