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

Language fixes to documentation #2293

Merged
merged 1 commit into from Jun 9, 2021
Merged

Language fixes to documentation #2293

merged 1 commit into from Jun 9, 2021

Conversation

ropg
Copy link
Contributor

@ropg ropg commented Jun 6, 2021

Hi there...

I like how it all works and how you documented it.

I very briefly went through more than half of the documentation now (still need to do porting and widgets) and fixed the obvious typos, broken sentences and grammar problems that I saw. I also changed a few things where I could see what was meant but it was not clear enough from the text.

Note that LV_OBJ_FLAG_SNAPABLE is misspelled: one would call this 'snappable' with double P. I made a note in the documentation to use a single P in code. Cleaner would be to change it everywhere and make LV_OBJ_FLAG_SNAPABLE and LV_OBJ_FLAG_SNAPPABLE synonyms in lv_obj.h so nothing breaks, but that's pretty deep inside the machine so I figured I'd ask first.

I'm happy to help further and more broadly when it comes to copy-writing and documentation...

My backstory: I spent a month and made (and then semi-abandoned) something remarkably like a mini-LVGL recently. (I tend to write things and only realize part-way in how much I need to add to make it as cool as I'd like it to be.) It did have multi-touch and ADSR multi-synth sound though... :)

Copy link
Member

@embeddedt embeddedt left a comment

Choose a reason for hiding this comment

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

Just a few minor nitpicks.

@@ -83,24 +83,24 @@ OR-ed values are also possible. E.g. `LV_DIR_TOP | LV_DIR_LEFT`.

### Scroll chain
If an object can't be scrolled further (e.g. it's content has reached the bottom most position) the scrolling is propagated to it's parent. If the parent an be scrolled in that direction than it will be scrolled instead.
It goes to the grad parent and grand grandparents too.
It propagets to the grandparent and grand-grandparents too.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
It propagets to the grandparent and grand-grandparents too.
It propagates to the grandparent and grand-grandparents too.

Comment on lines +183 to 186
- `LV_PART_MAIN` A background like rectangle*/``
- `LV_PART_SCROLLBAR` The scrollbar(s)
- `LV_PART_INDICATOR` Indicator, e.g. for slider, bar, switch, or the tick box of the checkbox
- `LV_PART_KNOB` Like a handle to grab to adjust the value*/
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- `LV_PART_MAIN` A background like rectangle*/``
- `LV_PART_SCROLLBAR` The scrollbar(s)
- `LV_PART_INDICATOR` Indicator, e.g. for slider, bar, switch, or the tick box of the checkbox
- `LV_PART_KNOB` Like a handle to grab to adjust the value*/
- `LV_PART_MAIN` A background like rectangle
- `LV_PART_SCROLLBAR` The scrollbar(s)
- `LV_PART_INDICATOR` Indicator, e.g. for slider, bar, switch, or the tick box of the checkbox
- `LV_PART_KNOB` Like a handle to grab to adjust the value

@@ -102,7 +102,7 @@ The core repositories have at least the following branches:
The changes are recorded in [CHANGELOG.md](/CHANGELOG).

### Version support
Before v8 every minor release of major releases are supproted for 1 year.
Before v8 every minor release of major releases is supported for 1 year.
Copy link
Member

Choose a reason for hiding this comment

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

I think the original sentence was incorrect.

Suggested change
Before v8 every minor release of major releases is supported for 1 year.
The version policy changed in v8. Before v8, the last minor release of each major release series was supported for 1 year. This means that 7.11 (which was released in March 2021) is supported till March 2022.
As of v8, each minor release is supported for 1 year.

Copy link
Contributor Author

@ropg ropg Jun 6, 2021

Choose a reason for hiding this comment

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

Yeah, thanks for clarifying. This was a case where I wasn't so sure what the original meant.

@ropg
Copy link
Contributor Author

ropg commented Jun 6, 2021

FYI: I added a commit (the porting directory) after approval..

@ropg ropg marked this pull request as draft June 7, 2021 10:42
@ropg ropg changed the title Language fixes part 1 Language fixes to documentation Jun 7, 2021
@ropg
Copy link
Contributor Author

ropg commented Jun 7, 2021

I converted this to draft while working the widgets. I've seen some spots where a second pass might be really helpful, but let's first give everything this first treatment of just correcting the typos, sentences, etc.

@ropg
Copy link
Contributor Author

ropg commented Jun 7, 2021

I need help with the meaning of the following paragraph:

### Auto-size
By default the Line's width and height is set to `LV_SIZE_CONTENT` to automatically set its size to involve all the points.
If the size if set explicitly the point out of the object
It can be enable with the `lv_line_set_auto_size(line, true)` function. 
If enabled then when the points are set the object's width and height will be changed according to the maximal x and y coordinates among the points. The *auto size* is enabled by default.

@embeddedt
Copy link
Member

I think this is correct for v8, though I'd love to hear @kisvegabor's feedback. lv_line_set_auto_size doesn't exist anymore because LV_SIZE_CONTENT does the same thing, as far as I know.

### Auto-size
By default the Line's width and height is set to `LV_SIZE_CONTENT`. This means it will automatically set its size to fit all the points.

If the size is set explicitly, some points may be clipped out of the line object.

@kisvegabor
Copy link
Member

Thank you very much, @ropg. It's so good that you have reviewed the docs. My English is far from perfects and the typos... 😄

I need help with the meaning of the following paragraph:

Probably I accidentally left half an old paragraph there. @embeddedt's suggestion is perfect.

@ropg
Copy link
Contributor Author

ropg commented Jun 8, 2021

### first line indent
Use `lv_spangroup_set_indent(spangroup, 20)` to set the indent of first line.

Is that in pixels?

@ropg
Copy link
Contributor Author

ropg commented Jun 8, 2021

OK, I just added docs/widgets/extra, which completes my language fixing pass over the documentation. I squashed the commits into a single one.

(I looked up the indent in code and specified that it's in pixels)

This by no means means there is nothing left to improve in the documentation. Technically it was already quite extensive and I did not see major problems that would cause people to truly not understand what is going on if they just read it twice. Now the language is clearer which will enhance immediate comprehension.

Now that I've read the whole thing: to make it truly pretty, it needs a style guide to determine, for instance, whether 'Object names' are capitalised and whether they have single quotes around them. Since the 'Object names' as referenced in text have spaces in them sometimes (Message box), I would suggest using italics or single quotes, because capitalising the first one only leaves the second part hanging too much. There's other inconsistencies like that, but such things take another complete pass and the value of this consistency to the reader is less that the changes in this pass.

The markdown has lot of newlines where the same paragraph continues. This is not a problem because the parser ignores it, it only makes the raw markdown a little harder to review. Also headers are sometimes followed by two newlines, sometimes by just one. (All really non-problems, could be fixed by - carefully - running a few regexp replaces.)

Hints for future documentation:

  • 'it' is not generally used in english sentences to refer back to the previous sentence right at the start of the sentence: in that case use 'this' instead.
  • 'tells' is not used in english in that way and was used for both get and set. 'sets', 'gets', 'is used to specify', 'can be used to obtain', are better and non-ambiguous.

Poster

I think what would be really cool is an LVGL programmer poster that has almost everything on it, which little pictures of all the widgets... In terms of easy lookup of stuff you use frequently, nothing beats an A0 poster on the wall right next to you.

@ropg
Copy link
Contributor Author

ropg commented Jun 8, 2021

Eh, conflicts... Will fix...

@ropg
Copy link
Contributor Author

ropg commented Jun 8, 2021

@kisvegabor ...

This commit takes out quite a bit of text while being titled "minor fix". Is that intentional?

@ropg
Copy link
Contributor Author

ropg commented Jun 9, 2021

@kisvegabor

Can you please respond so these can go in before more changes happen in the documentation?

@embeddedt
Copy link
Member

@ropg I reverted the color section removal in your branch for now, as otherwise your changes will be lost. I can merge this if you think it's ready. I don't see any major issues that can't be corrected after the fact. It's a huge improvement.

@ropg
Copy link
Contributor Author

ropg commented Jun 9, 2021

I can happily re-push with that section removed now that I know that's intended... Just a moment...

@kisvegabor
Copy link
Member

This commit takes out quite a bit of text while being titled "minor fix". Is that intentional?

It seems by mistake I committed it with another change. Sorry for the confusion.

I can happily re-push with that section removed now that I know that's intended... Just a moment...

Now "colors" has it's own page. It'd be great if you could take a look at it.

But - as you suggested - let's merge it to avoid conflicts in the future.

@kisvegabor kisvegabor marked this pull request as ready for review June 9, 2021 13:05
@ropg
Copy link
Contributor Author

ropg commented Jun 9, 2021

OK, conflicts resolved, everything ready...

@kisvegabor
Copy link
Member

Great, thank you very much for your amazing work!

@kisvegabor kisvegabor merged commit d0aaaca into lvgl:master Jun 9, 2021
@ropg
Copy link
Contributor Author

ropg commented Jun 9, 2021

The one unresolved issue here is what to do with LV_OBJ_FLAG_SNAPABLE, which needs to be snappable with double-P to be proper english.

Note that LV_OBJ_FLAG_SNAPABLE is misspelled: one would call this 'snappable' with double P. I made a note in the documentation to use a single P in code. Cleaner would be to change it everywhere and make LV_OBJ_FLAG_SNAPABLE and LV_OBJ_FLAG_SNAPPABLE synonyms in lv_obj.h so nothing breaks, but that's pretty deep inside the machine so I figured I'd ask first.

@kisvegabor
Copy link
Member

I've just fixed it here: e697807

@ropg
Copy link
Contributor Author

ropg commented Jun 10, 2021

Shouldn't docs.lvgl.io on readthedocs point to the latest documentation? Not only now because we did all the fixes, but more generally: I think it's much more likely that the documentation in master is better vs the odds that it's "ahead".

@embeddedt
Copy link
Member

@ropg Because the copy of lvgl.h in master still contains 8.0 as the version, the changes made to documentation on master are reflected on the 8.0 URL.

For example, this section contains the updated sentence "TFT_eSPI is used as the display driver."

@kisvegabor
Copy link
Member

I think @ropg meant that docs.lvgl.io should be redirected to https://docs.lvgl.io/master/ instead of https://docs.lvgl.io/8.0/index.html which I think is a good idea.

@embeddedt
Copy link
Member

Okay; done here.

@kisvegabor
Copy link
Member

Because the copy of lvgl.h in master still contains 8.0 as the version, the changes made to documentation on master are reflected on the 8.0 URL.

BTW, I think we should increment to the version number to v8.1.0-dev. So you agree?

@embeddedt
Copy link
Member

Okay, but we should probably merge master into release/v8.0 and do a patch release first. That way we don't have to cherry pick things later.

@kisvegabor
Copy link
Member

We need to cherry-pick commits any way because there are features anyway.
I'll release v8.0.1 today to have a cleaner state on master.

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.

None yet

3 participants