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

[craftedv2beta] Add documentation for crafted-completion #331

Merged
merged 3 commits into from
Aug 25, 2023
Merged

[craftedv2beta] Add documentation for crafted-completion #331

merged 3 commits into from
Aug 25, 2023

Conversation

sthesing
Copy link
Contributor

I adapted the docs for crafted-completion that I originally wrote for v1. Docs for Corfu and Cape included.

I hope the original approach with explaining the packages step-by-step with screenshots is still alright.

@jvdydev
Copy link
Contributor

jvdydev commented Aug 18, 2023

Thank you, this already looks really good.

I wonder if instead of having a "Documentation" heading, instead having a heading for each package (given the in-depth explanations).
I assume you tried to adapt to what the other docs already have (which I appreciate), however I think this would be a case where breaking things out may help.

Also some very small polishing things like typos or accidental characters (not meant negatively, just something I noticed. I would be doomed without spell checkers).

@sthesing
Copy link
Contributor Author

I have introduced subheadings now, is this what you head in mind?
I'm doomed without spell checking, too, and even with the help of it, I found only one typo. If you spotted more, I'd be grateful for pointing me towards them.
Thanks for your feedback!

@jeffbowman
Copy link
Contributor

Screenshots are nice to tell a story, but we only have them for this one configuration. It's harder to imagine screenshots for all other modules, but in the interest of consistency, are there any other modules we should consider a similar approach?

@sthesing
Copy link
Contributor Author

Screenshots are nice to tell a story, but we only have them for this one configuration. It's harder to imagine screenshots for all other modules, but in the interest of consistency, are there any other modules we should consider a similar approach?

I did consider them while drafting the documentation for crafted-ui, too (PR coming soon). My first thought was that crafted-completion and crafted-ui affect how the interface looks, so I thought both might benefit from screenshots.
However, the look aspect is not so much the topic here, it's more a matter of positioning: where does Emacs put which information presented to the user?
In that, crafted-completion is rather special, so I do think screenshots are helpful, here.
But, as I wrote back in #128 and #155, I tried to write the text so that it works without screenshots, too. (Not sure if I succeeded, though...)

@jeffbowman
Copy link
Contributor

Yep, and I'm a fan of the screenshots, just wanted to ask if there were other places they made sense as well.

@sthesing
Copy link
Contributor Author

just wanted to ask if there were other places they made sense as well.

I've been thinking about this a bit, and have toyed with the idea whether it might be worthwhile to add screenshots to the documentation of crafted-screencast or crafted-workspaces. But just as with crafted-ui, I feel that in both cases the brevity of pure text is preferable to using screenshots. What do you think?

Do you have any module in mind where it might be worth looking into the benefit of screenshots?

P.S.: I merged the docs for crafted-writing and regenerated the info file, so this should be mergable now.

@jvdydev jvdydev mentioned this pull request Aug 23, 2023
17 tasks
@jeffbowman
Copy link
Contributor

Still seeing merge conflicts on this PR, which is the only reason it has not been merged.

@jvdydev
Copy link
Contributor

jvdydev commented Aug 25, 2023

weird, says "This branch has no conflicts with the base branch" for me.
Should I try merging?

@jeffbowman
Copy link
Contributor

Sure, give it a shot!

@sthesing
Copy link
Contributor Author

Still seeing merge conflicts on this PR, which is the only reason it has not been merged.

Strange, I also don't see conflicts.

Thanks, for trying @jvdydev . If I did something wrong, give me a holler...

@jvdydev jvdydev merged commit d2b90f0 into SystemCrafters:craftedv2beta Aug 25, 2023
@jvdydev
Copy link
Contributor

jvdydev commented Aug 25, 2023

I think it was confused because the commits are older than some of the other PRs that already got merged (hence there was a lot of rebasing different commits to keep the timeline accurate)?

Anyway, seems like it worked

@sthesing sthesing deleted the v2b-doc-completion branch August 25, 2023 17:26
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