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

Add support for deprecated property #1240

Merged
merged 1 commit into from Oct 21, 2021
Merged

Conversation

yyoncho
Copy link
Contributor

@yyoncho yyoncho commented Oct 20, 2021

No description provided.

company.el Outdated Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
@dgutov dgutov merged commit c7bc412 into company-mode:master Oct 21, 2021
@dgutov
Copy link
Member

dgutov commented Oct 21, 2021

Thanks!

@dgutov
Copy link
Member

dgutov commented Oct 21, 2021

@yyoncho Looking at the gif you just posted: how about we apply the strike-through effect (and the deprecation face, I guess) only to the textual part of the completion item? Meaning, not the margins or the whitespace.

Like on VS Code's screenshot here:

https://code.visualstudio.com/assets/updates/1_38/css-deprecated-properties.png

@yyoncho
Copy link
Contributor Author

yyoncho commented Oct 22, 2021

I don't have a strong opinion on that.

dgutov added a commit that referenced this pull request Oct 23, 2021
To avoid bringing in its background and etc.

#1240
@dgutov
Copy link
Member

dgutov commented Oct 23, 2021

OK, I've done the change. Looks a bit more subtle.

Bonus news: elisp-completion-at-point now supports deprecated on master (Emacs 29).

@yyoncho
Copy link
Contributor Author

yyoncho commented Oct 23, 2021

Bonus news: elisp-completion-at-point now supports deprecated on master (Emacs 29).

Maybe it is time to put it in elpa

@dgutov
Copy link
Member

dgutov commented Oct 23, 2021

Might be a pain to rewrite it in a way that works for Emacs 26.

If you really want this feature in Emacs 28, though, I can try to initiate a backport.

@yyoncho
Copy link
Contributor Author

yyoncho commented Oct 23, 2021

Is having elpa package that supports only emacs 27+(or even only emacs 28) an option? It is not big deal for me, but IMO it is very unfortunate that a handy feature like that will be available to the users after 2(?)+ years.

@dgutov
Copy link
Member

dgutov commented Oct 23, 2021

Any kind of long-term support is additional maintenance burden. I think over time we will add more and more packages to "ELPA core", but I wish we get some better CI infrastructure by then, including running the tests of said packages in the oldest supported version.

Just over the recent months, I've managed to break project.el's and xref's Emacs 26 compatibility, and people are depending on it now. So that wasn't nice.

@dgutov
Copy link
Member

dgutov commented Oct 23, 2021

A backport has much fewer risks, so if you try the new change in Emacs 29 for a couple of days and like it, doing that seems optimal.

The current implementation will say a variable is deprecated even if only a function with the same name is (or a face, etc). So either we verify it's no big deal, or it needs to be improved first.

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

2 participants