updates to the analysis server generated spec doc

Minor changes to the generated spec, and display deprecated methods better.

BUG=
R=brianwilkerson@google.com, scheglov@google.com

Committed: https://github.com/dart-lang/sdk/commit/eb1839b8fd453b6c0c93376346743fa96db48440

[devoncarew, 2017-04-08 20:49:12]

Description was changed from

==========
dartfmt


re-gen spec doc


minor changes to the generated spec, and display deprecated methods better

BUG=
==========

to

==========
Minor changes to the generated spec, and display deprecated methods better.

BUG=
==========

[devoncarew, 2017-04-08 20:52:31]

devoncarew@google.com changed reviewers:
+ brianwilkerson@google.com, scheglov@google.com

[devoncarew, 2017-04-08 20:52:32]

Some updates to the generated spec doc:
- represent deprecated symbols with strikethrough css, and make sure the
deprecated data shows up in all places
- add request methods to the TOC at the start of the generated spec
- some additional small tweaks to the generated spec

[Brian Wilkerson, 2017-04-09 16:30:53]

https://codereview.chromium.org/2800283002/diff/1/pkg/analysis_server/lib/plu...
File pkg/analysis_server/lib/plugin/protocol/generated_protocol.dart (right):

https://codereview.chromium.org/2800283002/diff/1/pkg/analysis_server/lib/plu...
pkg/analysis_server/lib/plugin/protocol/generated_protocol.dart:9177: *
@deprecated: this feature is always enabled.
Personally, I like the old style better. I don't think the at sign adds any
value either here or in the specification, despite the similarity to the
annotation. This isn't Dart; it just adds one more special character to have to
parse while reading it. You replaced a pair or parentheses with a colon, which I
assume is to make the text visually less bulky, so I don't understand adding an
extra character here when it has the opposite effect.

That said, it would be interesting to consider adding the actual annotation to
the generated code:

/**
 * Deprecated: this feature is always enabled.
 *
 * True if the client wants to enable support for the proposed async feature.
 */
@deprecated
bool get enableAsync => _enableAsync;

This might result in a lot of extraneous hints in the analysis_server package,
though, because we still have to reference these members until we actually
remove them from the API.

https://codereview.chromium.org/2800283002/diff/1/pkg/analysis_server/test/in...
File pkg/analysis_server/test/integration/integration_test_methods.dart (right):

https://codereview.chromium.org/2800283002/diff/1/pkg/analysis_server/test/in...
pkg/analysis_server/test/integration/integration_test_methods.dart:148: *
analysis: AnalysisStatus (@optional)
I really don't think the at sign adds anything either here or in the spec. Could
we remove it and reduce the visual noise?

https://codereview.chromium.org/2800283002/diff/1/pkg/analysis_server/tool/sp...
File pkg/analysis_server/tool/spec/spec_input.html (left):

https://codereview.chromium.org/2800283002/diff/1/pkg/analysis_server/tool/sp...
pkg/analysis_server/tool/spec/spec_input.html:122: </p>
This really *does* need to be documented, so I'd prefer if we didn't loose the
reminder.

[devoncarew, 2017-04-09 19:43:21]

OK, updated, PTAL.

I moved back to using 'Deprecated:' in the generated docs, but did try the idea
of using @deprecated annotations in the generated libraries. I only have to make
a few test locations to ignore deprecated symbol usage. Definitely having the
@deprecated annotations in the generated Java library is a net win - it's a
clear message to API consumers about when things are no longer supported or
recommended.

https://codereview.chromium.org/2800283002/diff/1/pkg/analysis_server/lib/plu...
File pkg/analysis_server/lib/plugin/protocol/generated_protocol.dart (right):

https://codereview.chromium.org/2800283002/diff/1/pkg/analysis_server/lib/plu...
pkg/analysis_server/lib/plugin/protocol/generated_protocol.dart:9177: *
@deprecated: this feature is always enabled.
On 2017/04/09 16:30:53, Brian Wilkerson wrote:
> Personally, I like the old style better. I don't think the at sign adds any
> value either here or in the specification, despite the similarity to the
> annotation. This isn't Dart; it just adds one more special character to have
to
> parse while reading it. You replaced a pair or parentheses with a colon, which
I
> assume is to make the text visually less bulky, so I don't understand adding
an
> extra character here when it has the opposite effect.
> 
> That said, it would be interesting to consider adding the actual annotation to
> the generated code:
> 
> /**
>  * Deprecated: this feature is always enabled.
>  *
>  * True if the client wants to enable support for the proposed async feature.
>  */
> @deprecated
> bool get enableAsync => _enableAsync;
> 
> This might result in a lot of extraneous hints in the analysis_server package,
> though, because we still have to reference these members until we actually
> remove them from the API.

Sure, I moved back to the 'Deprecated:' text. Will experiment with actual
@deprected annotations in the generated libraries.

> You replaced a pair or parentheses with a colon

That change was because the properties looked a bit like method declarations to
me; I think the 'foo: String' rendering is a little less confusing.

https://codereview.chromium.org/2800283002/diff/1/pkg/analysis_server/test/in...
File pkg/analysis_server/test/integration/integration_test_methods.dart (right):

https://codereview.chromium.org/2800283002/diff/1/pkg/analysis_server/test/in...
pkg/analysis_server/test/integration/integration_test_methods.dart:148: *
analysis: AnalysisStatus (@optional)
On 2017/04/09 16:30:53, Brian Wilkerson wrote:
> I really don't think the at sign adds anything either here or in the spec.
Could
> we remove it and reduce the visual noise?

Changed to use '(optional)'.

https://codereview.chromium.org/2800283002/diff/1/pkg/analysis_server/tool/sp...
File pkg/analysis_server/tool/spec/spec_input.html (left):

https://codereview.chromium.org/2800283002/diff/1/pkg/analysis_server/tool/sp...
pkg/analysis_server/tool/spec/spec_input.html:122: </p>
On 2017/04/09 16:30:53, Brian Wilkerson wrote:
> This really *does* need to be documented, so I'd prefer if we didn't loose the
> reminder.

Probably better to track in an issue than in our published docs though? I added
a blurb with what I think may be a good stb at some text; let me know how it
looks.

[Brian Wilkerson, 2017-04-10 13:45:17]

lgtm, thanks!

Could you copy the changes to the generator to the copy of the code that's in
'analyzer_plugin'? I had to make a few changes to the plugin version, but I'd
like to keep the two versions as similar as possible until I (or someone) has
time to unify them.

https://codereview.chromium.org/2800283002/diff/1/pkg/analysis_server/tool/sp...
File pkg/analysis_server/tool/spec/spec_input.html (left):

https://codereview.chromium.org/2800283002/diff/1/pkg/analysis_server/tool/sp...
pkg/analysis_server/tool/spec/spec_input.html:122: </p>
> Probably better to track in an issue than in our published docs though?

Probably not, because of the huge number of issues. I suspect we'd never find
the issue again. :-)

> I added a blurb with what I think may be a good stb at some text; let me
> know how it looks.

It's much better than what we had!

[scheglov, 2017-04-10 13:54:52]

LGTM

[devoncarew, 2017-04-10 15:17:51]

> Could you copy the changes to the generator to the copy of the code that's in
'analyzer_plugin'? I had to make a few changes to the plugin version, but I'd
like to keep the two versions as similar as possible until I (or someone) has
time to unify them.

I made changes to 5 dart files in tool/spec. Do you mean all of them or a
specific sub-set?

It looks like there's about ~3000 lines of dart in analyzer_plugin/tool/spec. We
should determine the diffs from the analysis server version and unfork.

[Brian Wilkerson, 2017-04-10 15:43:28]

> I made changes to 5 dart files in tool/spec. Do you mean all of them or a
> specific sub-set?

Any of them that were copied to analyzer_plugin.

> It looks like there's about ~3000 lines of dart in analyzer_plugin/tool/spec.
We
> should determine the diffs from the analysis server version and unfork.

I agree, but it's a question of priorities. There are very few changes to the
copied files, so it will largely be a matter of converting server to use the
implementation in plugin. The primary reason the files were copied is because
plugin can't reference code in server; I just wasn't able to finish the job of
removing the files from server.

[devoncarew, 2017-04-15 19:48:18]

> Any of them that were copied to analyzer_plugin.

I was asking which files.

> The primary reason the files were copied is because plugin can't reference
code in server

The code is in the tools/ directory, so server won't be able to see the code
either. We either want to have a separate package w/ the code in lib/, and have
both server and plugins add that package as a dev dependency, or just use
relative paths to access the source between plugins, as the code will always
live in the sdk repo.

[devoncarew, 2017-04-15 20:43:39]

Description was changed from

==========
Minor changes to the generated spec, and display deprecated methods better.

BUG=
==========

to

==========
Minor changes to the generated spec, and display deprecated methods better.

BUG=
R=brianwilkerson@google.com, scheglov@google.com

Committed:
https://github.com/dart-lang/sdk/commit/eb1839b8fd453b6c0c93376346743fa96db48440
==========

[devoncarew, 2017-04-15 20:43:41]

Committed patchset #4 (id:60001) manually as
eb1839b8fd453b6c0c93376346743fa96db48440 (presubmit successful).