Add crossed out modifier for deprecated diagnostics

[jpaju]

Render deprecated symbols as crossed out.

Before:

After:

[nik-rev]

I am personally against this idea, since it can make the text harder to read if its crossed out. Many codebases may have deprecated functions still being used, and it may not be possible to instantly upgrade them.

Your compiler / IDE should give you warnings about deprecated methods, not syntax highlighting. I can understand this change for intellisense suggestions, but if deprecated methods are always crossed out it makes code harder to read

[jpaju]

Many codebases may have deprecated functions still being used, and it may not be possible to instantly upgrade them.

That's true, but I'd argue it's good to see at a glance when some symbols are deprecated. Not sure I completely understood the argument on why we shouldn't have this visual indication on deprecated symbols. Could you elaborate on it?

Your compiler / IDE should give you warnings about deprecated methods, not syntax highlighting. I can understand this change for intellisense suggestions, but if deprecated methods are always crossed out it makes code harder to read

Hmm, what do you mean by intellisense suggestions ? The language server informs editor which symbols are deprecated as part of diagnostics response. Then the editor decides how to act on this information. VS code and IntelliJ for example seem to use strikethrough.

Other themes in Helix seem to systematically have deprecated symbols crossed out. If one doesn't like this, maybe there could be some configuration option in Helix or you could use custom theme?

[backwardspy]

i'm also a little unsure. i personally find text with strikethrough hard to read with some fonts & terminal emulators. there's also already a bunch of decoration making the deprecation warning obvious.

on the other hand, as you've pointed out this is seemingly common to most (though not all) of the bundled helix themes, so users are presumably already used to seeing it.

as far as i know there's no way to set options for a theme, so we'd have to distribute another set of theme files like we do for the default and no_italics variants at the moment. i'm unwilling to add more combinations as it'll quickly grow confusing and unmanageable. if users had a way to easily set whether they wanted this enabled or not i'd be much happier with it.

i don't know yet which way to go. i'm gonna use it for a while and see how i feel, in the meantime please do continue to provide your perspectives! i find it valuable to hear from both sides.

example with the default theme for reference:

[nik-rev]

intellisense is another word for the autocompletion you get from the language server

What I mean is that with these autocompletions, it's fine to have the specific autocompletions crossed out. For instance the description() below is deprecated:

why its fine: New code written shouldn't use deprecated methods. It's a handy indicator that despite this being a valid API, you shouldn't use it

I'd argue it's good to see at a glance when some symbols are deprecated

You should be able to achieve that with the LSP warning. For instance here, the method generates a warning for being deprecated

But if you cross out the description, it'll make it harder to read at no extra benefit. You already have the information that this method is deprecated from a warning, crossing it out would just make it harder to read