Encourage tools to parse a @Deprecated message like a doc comment

[srawlins]

The idea here is to allow (and encourage) users to write a @Deprecated message as if it were a doc comment, using square brackets to refer to elements, and to use markdown if desired.

Here's an example of a @Deprecated parameter in Flutter: https://api.flutter.dev/flutter/gestures/GestureRecognizer/GestureRecognizer.html

Rather than try to figure out how to rewrite the two adjacent strings, and account for different string delimiters, raw-ness, multiline delimiters, etc. I think it would be a great feature to take the String value, (which would not include delimiters, etc), and then render it like a doc comment: allow the developer to put supportedDevices in square brackets.

Then in the IDE, you could go-to-definition, see hover text, etc. Dartdoc would also cross-link.

I admit this would be very specific handling of exactly one annotation, but I think the benefit to users really outweighs whatever downside that amounts to.

CC @bwilkerson @goderbauer @gspencergoog who might want to weigh in.

[bwilkerson]

It's kind of a cool idea, but ...

Special casing always has a cost; if nothing else in all the other places where users then expect special handling and don't get it. If we were going to do something like this, then I would advocate for something like a @markdown annotation that can be applied to constructor parameters on const classes being used as annotations.

But, I have a concern about supporting something like this at all. We have at least two annotations (this being one) in which the argument to the constructor is also used in the diagnostic message that's produced by the analyzer. That means that the string will be displayed in IDEs where markdown isn't supported, and I'm concerned about the readability impact of having unprocessed markdown in diagnostic messages. In order to make this proposal acceptable I think we'd need to process the text and convert it to plain text as part of generating diagnostics, and I don't know what the performance implications of that would be (or whether there would be text that would be difficult to convert well, such as nested lists).

[srawlins]

I would advocate for something like a @markdown annotation that can be applied to constructor parameters on const classes being used as annotations.

I could get behind that.

That means that the string will be displayed in IDEs where markdown isn't supported, and I'm concerned about the readability impact of having unprocessed markdown in diagnostic messages.

I don't think this is a problem. In the original philosophy of Markdown, Gruber writes:

Markdown is intended to be as easy-to-read and easy-to-write as is feasible.

Readability, however, is emphasized above all else. A Markdown-formatted document should be publishable as-is, as plain text, without looking like it’s been marked up with tags or formatting instructions.

This makes it the perfect format for reading doc comments as plain text (not rendered into HTML), and for reading deprecation messages as plain text. Any square brackets, asterisks or underscores which are not understood, should not hurt the readability very much. If it became a big thorn, we can also render to HTML and then strip HTML. **Please** use [otherThing] _instead_ can just become "Please use otherThing instead" in a printed diagnostic message.

[goderbauer]

I do like the idea. In my reviews I have always been encouraging people to put the text from the deprecated annotation also in the doc comment itself, precisely because in the doc comment you can link to things mentioned in the message that provide more context. Something you currently can't do in the deprecation message itself. If that would get fixed, we could avoid this duplication.

[bwilkerson]

... and for reading deprecation messages as plain text.

Whether they succeeded in their intentions is a question I'll ignore for the moment. But I'm somewhat familiar with what markdown looks like, and I think that embedded links could still be a problem, both in terms of readability and in terms of line length (most of the locations where messages are displayed in IDEs are space constrained). List could also be problematic when line breaks are removed.

... precisely because in the doc comment you can link to things mentioned in the message that provide more context.

That would presumably be true in dart doc produced output, but would not be true in the IDEs.

The situation wouldn't be particularly worse than it is today except for the fact that the message would likely be written to assume a level of support that won't always be available. For example a message like "You can't do that. See this doc for more detail." wouldn't be as helpful if we dropped the URL from the output. Of course, I don't know whether the arguments to the constructor are currently written to work well without formatting, so maybe it won't make any difference at all.

[srawlins]

List could also be problematic when line breaks are removed.

For example a message like "You can't do that. See this doc for more detail." wouldn't be as helpful if we dropped the URL from the output.

Good points. I think my idea to strip HTML would not work. Much better to leave the markdown text as unparsed.

Anyhow, this change would not encourage anyone to make a longer deprecation message than they do today. If today you write a multiline string, or you write the text "please go see http://flutter.dev/... for details on how...", it displays just as well/poorly in diagnostic messages. Encouraging markdown only improves UX in the IDE and dartdoc, and maybe introduces some square brackets in diagnostic messages.

[bwilkerson]

Encouraging markdown only improves UX in the IDE ...

I disagree. If you don't design for a targeted environment there's a good chance that the resulting UX will be poorer than if you did. In my opinion, markdown is good when it's processed, and tolerable in some cases where it isn't, but very poor in a minority of cases. I believe that this is one of the minority cases where it will be poor.

I just don't know how to convince you that I'm right. :-)

Personally, I think that

Try doing one of the following: - the first alternative, - the second alternative, or - the third alternative.

is strictly worse than

Try doing one of the following: the first alternative, the second alternative, or the third alternative.

It isn't completely unreadable (though I suspect it wouldn't be hard to make it much closer to unreadable by adding more markdown), but it isn't optimal. I'm just asking that if we're going to implement this feature that we consider ways to make the UX as close to optimal as possible.

[srawlins]

I 100% agree that the first in your case is strictly worse than the second. I don't get the connection though. Why would someone write the first?

[bwilkerson]

By writing

Try doing one of the following:
- the first alternative,
- the second alternative, or
- the third alternative.

and having the tool concatenate the lines. I guess I was assuming that we wouldn't just truncate to the first line, because that seems even worse.

[srawlins]

I see. Today, someone can write that message, and if a bulleted list is the best way to help the user get around the deprecation, then I would encourage it today. I would not encourage someone to avoid the best text because it won't look nice in a command line diagnostic. There are messages in flutter's code base like:

@Deprecated(
  'This property is no longer used, please use toolbarTextStyle and titleTextStyle instead. '
  'This feature was deprecated after v2.4.0-0.0.pre.',
)

which is really long, and might not fit into a hover dialog in an IDE, but I wouldn't suggest that they cut the text and make the text less useful.

If someone was going to write a multi-line Deprecated message with text that looks like a list, why would they format it one way or another based on whether markdown formatting is encouraged? Why would someone not write that message today, but they would if we encouraged markdown formatting?

[bwilkerson]

I really shouldn't trust my memory. :-( Much of this conversation is moot. I should have double checked earlier, but it's been a rough day. I apologize.

I tested both the long message and the list, as well as some other markdown. The long message wraps in both VS Code and IntelliJ (in hovers) and is quite readable. Not so much in the problems view in IntelliJ, though it also displays fine in the problems view in VS Code. The line breaks in the list are preserved, and both display just fine in hovers and in VS Code's problems view, but not so nice in IntelliJ. Markdown is not processed, however, so other markdown is less readable.

While I still think we can do better if we remove some of the markdown when displaying the message in IDEs (and on the command-line), it's not nearly as bad as I thought it would be.

I did find another way to improve our handling of these messages. We should really trim them and we shouldn't add a period after the inserted message if it already ends with punctuation.

[srawlins]

I should have double checked earlier, but it's been a rough day. I apologize.

No problem! I was definitely also not checking. I 100% assumed the hovers would collapse newlines. Thanks for doing the checking work! I should have looked into that.

While I still think we can do better if we remove some of the markdown when displaying the message in IDEs (and on the command-line), it's not nearly as bad as I thought it would be.

Yeah this is interesting. I can't think of any great examples, but I'm sure you're right. For example, in @Deprecated('**Never** use this'), I think the text should be preserved with asterisks or underlines that indicate "emphasis." Many people do this in plain text. A blocks structure like a list or a blockquote is extremely weird... I just don't think I've ever seen a multi-line deprecation message, so I don't know how we would treat it... maybe collapsing some syntax.

It might depend on whether IDEs have auto-linking capabilities too. Like if an IDE were to render "hello http://google.com" in a hover, does it auto-link?

[bwilkerson]

Given

/// hello http://google.com
void f() {}

In IntelliJ, the URL does not become linkable in the hover text, but it is linked in the dartdoc comment.

In VS Code, the URL becomes linkable in both contexts.

Given

/// **Never** use this
void f() {}

Both do nothing in the editor, but do translate to bold text in the hover.

[srawlins]

CC @lrhn (or please CC the relevant party if this should go to someone else)

As Deprecated is part of dart:core, we should get some approval from owners of the core libraries. WDYT of the proposal at the top here, @lrhn ? There would be nothing to implement in the core libraries, or language. Implementation would lie in analyzer and dartdoc.

[lrhn]

I'd leave this decision to the tool authors.

If DartDoc, the analyzer, and the analysis server (which sends diagnostics to IDEs) think they can provide a better user experience by allowing and supporting markdown, then I'm all for it. If they fear it will be worse, then I'd trust them to not support markdown.

I don't want to add extra parameters to Deprecated, like an optional named markdown parameter. Writing things once should be enough.

Consider instead to treat a paragraph of the DartDoc for the feature which starts with Deprecated: or Deprecation: as the paragraph to display. We can make that the canonical way to document deprecation, if it's easier for everybody. (But so far documentation paragraphs starting with "Returns" or "Throws" have not been recognized as documenting the return value or exceptions, like they were intended to, so we don't have a good tradition for showing partial dartdoc content.)