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

Separate tasks from regular modules #713

Merged
merged 4 commits into from May 24, 2017

Conversation

michalmuskala
Copy link
Member

@michalmuskala michalmuskala commented May 16, 2017

  • separate tasks in html output
  • separate tasks in epub output
  • html tests
  • js tests
  • epub tests
  • change page title for tasks from Mix.Tasks.Foo.Bar to foo.bar

Closes #712

@michalmuskala
Copy link
Member Author

michalmuskala commented May 17, 2017

Here's how it looks in phoenix right now:

screenshot 2017-05-17 07 40 38

The URL of the page is exactly the same - with the full module name. I don't think it's worth it to change it considering we would break all the bookmarks and links doing so.

end
# assert Templates.synopsis(doc2) ==
# "Example function: Summary should not display trailing puntuation"
# end
Copy link
Member Author

Choose a reason for hiding this comment

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

Moving the synopsis generation to the retriever means we can't test it as easily now.

I'm not 100% sure about this move. Maybe it's fine to use the first line of the moduledoc for tasks as well instead of the shortdoc. It's only used on the "API Reference" page, which is, in my experience, used rather rarely.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, let's revert the synopsis change for now. We can always add the feature if necessary later.

@whatyouhide
Copy link
Member

I would use "Mix task" as the label on the right of the name instead of "task" to be clearer.

@josevalim
Copy link
Member

@whatyouhide tasks is fine as you can get the context as soon as you click on the first one of them. My only suggestion is to change the title on the right side to be mix phx.gen.html (task) instead of phx.gen.html (task). Then I think we are covered from adding mix to the sidebar. What do you think?

@michalmuskala
Copy link
Member Author

Should we also change the name in the summary on the left?

@michalmuskala
Copy link
Member Author

I updated it (not pushed yet) to list mix foo instead of foo task as the title:

screenshot 2017-05-18 11 46 25

@josevalim
Copy link
Member

@michalmuskala let's keep mix in bold as well, basically as part of the title. Is that possible?

@michalmuskala
Copy link
Member Author

Anything's possible if you try hard enough 😏

I'll change it.

@michalmuskala
Copy link
Member Author

@josevalim It would look like this:

screenshot 2017-05-18 11 57 18

Or do we want to keep the small "task" there as well?

@michalmuskala
Copy link
Member Author

Another question is the page title - should it also include mix? And the search page. Currently it looks like this:
screenshot 2017-05-18 12 05 42

@josevalim
Copy link
Member

Or do we want to keep the small "task" there as well?

Up to you. It doesn't feel it is necessary though.

Another question is the page title - should it also include mix?

Up to you. It doesn't feel it is necessary though.

@josevalim
Copy link
Member

Let us know when this is ready to ship so we can release a new version. :)

functions: [
{id: 'hello world', anchor: 'hello-world'}
]
}, {
id: 'world2',
id: 'world2', title: 'world2',

Choose a reason for hiding this comment

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

Object properties must go on a new line if they aren't all on the same line.

functions: [
{id: 'hello world', anchor: 'hello-world'}
]
}, {
id: 'world',
id: 'id2', title: 'world',

Choose a reason for hiding this comment

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

Object properties must go on a new line if they aren't all on the same line.

@@ -12,30 +13,30 @@ describe('search', () => {

it('searches for function matches', () => {
var nodes = [{
id: 'hello world',
id: 'id1', title: 'hello world',

Choose a reason for hiding this comment

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

Object properties must go on a new line if they aren't all on the same line.

@michalmuskala michalmuskala changed the title [WIP] Separate tasks from regular modules Separate tasks from regular modules May 24, 2017
@michalmuskala
Copy link
Member Author

This is ready for review

@josevalim josevalim merged commit ccd2be0 into elixir-lang:master May 24, 2017
@josevalim
Copy link
Member

❤️ 💚 💙 💛 💜

@whatyouhide
Copy link
Member

\cc @Kapeli, heyo, I think this will affect how documentation is structured in Dash: there's now a new "Mix Tasks" section alongside Protocols, Exceptions, and so on. May be worth mirroring that in Dash.app :)

@Kapeli
Copy link

Kapeli commented May 24, 2017

Thanks @whatyouhide, I'll make sure Dash works well with this change.

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

Successfully merging this pull request may close these issues.

None yet

4 participants