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.

Sign up for GitHub

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

Jump to bottom

Fixed breaking changes after company-mode update #168

Merged
merged 1 commit into from
Jul 9, 2021
Merged

Fixed breaking changes after company-mode update #168

merged 1 commit into from
Jul 9, 2021
+15 −15

Conversation

CsBigDataHub
Copy link
Contributor

@CsBigDataHub CsBigDataHub commented Jul 9, 2021 •
edited
Loading

fixes #167

  • Fixed warning for dash.

company-mode updated functions

  • company-show-numbers to company-quick-access
  • company-show-numbers-function to company-quick-access-hint-function

Fixed warning for dash. company-mode updated functions
4d2b049 
- company-show-numbers changed to company-quick-access
- company-show-numbers-function to company-quick-access-hint-function
@CsBigDataHub CsBigDataHub mentioned this pull request Jul 9, 2021
Closed
@seagle0128
Copy link

ping @sebastiencs ...

It's a critical fix.Please merge it ASAP. Thanks!

@sebastiencs sebastiencs merged commit c8a8671 into sebastiencs:master Jul 9, 2021
@sebastiencs
Copy link
Owner

Merged, thanks @CsBigDataHub @seagle0128

@CsBigDataHub
Copy link
Contributor Author

CsBigDataHub commented Jul 9, 2021 •
edited
Loading

Thanks @sebastiencs .

Now the numbering is off, when show-numbers is enabled.

image

I suspect these line is the culprit -

company-box/company-box.el

Line 293 in c8a8671

(string-trim (funcall company-quick-access-hint-function (mod (1+ index) 10)))

or

company-box/company-box.el

Line 411 in c8a8671

(put-text-property (1- it) it 'display `((margin ,side) ,(aref company-box--numbers (+ index offset))))))))

@dgutov
Copy link

dgutov commented Jul 9, 2021

Try removing (+1 from the first one.

CsBigDataHub added a commit to CsBigDataHub/company-box that referenced this pull request Jul 9, 2021
@CsBigDataHub
Fix numbering when company-numbers is enabled
7908e51 
Fixed sebastiencs#168 (comment)
@CsBigDataHub CsBigDataHub mentioned this pull request Jul 9, 2021
Merged
@CsBigDataHub
Copy link
Contributor Author

@dgutov thanks
Created a PR #169

@sebastiencs please push it whenever you can.

@seagle0128
Copy link

Another thing is company-box numbers should respect company-tooltip-quick-access face.

wyuenho pushed a commit to wyuenho/company-box that referenced this pull request Jul 10, 2021
@CsBigDataHub @wyuenho
Fix numbering when company-numbers is enabled
f62d8c0 
Fixed sebastiencs#168 (comment)
sebastiencs pushed a commit that referenced this pull request Jul 12, 2021
@CsBigDataHub @sebastiencs
Fix numbering when company-numbers is enabled
156f65c 
Fixed #168 (comment)
basil-conto
basil-conto reviewed Jul 27, 2021
View reviewed changes
company-box.el
@@ -62,7 +62,7 @@

(require 'subr-x)
(require 'dash)
(require 'dash-functional)
(require 'dash)
Copy link
Contributor

Choose a reason for hiding this comment

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

There is already (require 'dash) in this list (see PR #170).

More importantly, you can't replace dash-functional with dash without bumping the required Dash version to at least 2.18.0 (see PR #152).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the PR, I guess I did not realize dash version in here as it is a dependency for multiple packages and it's already updated to latest version. Probably @sebastiencs can find time to merge your PR's soon.

@basil-conto basil-conto mentioned this pull request Jul 27, 2021
Merged
sebastiencs pushed a commit that referenced this pull request Oct 20, 2021
@basil-conto @sebastiencs
Fix Dash dependencies
f9cbbc7 
Dash 2.18.0 subsumed dash-functional, and company-box no longer
loads dash-functional, but it still lists old versions of dash and
dash-functional as dependencies, which can cause issues.

While at it, require the latest version 2.19.0 of Dash.

Re: #152, #153, #156, #157, #161, #168, #170.
iris-garcia pushed a commit to iris-garcia/company-box that referenced this pull request Oct 25, 2021
@basil-conto @iris-garcia
Fix Dash dependencies
ea317cb 
Dash 2.18.0 subsumed dash-functional, and company-box no longer
loads dash-functional, but it still lists old versions of dash and
dash-functional as dependencies, which can cause issues.

While at it, require the latest version 2.19.0 of Dash.

Re: sebastiencs#152, sebastiencs#153, sebastiencs#156, sebastiencs#157, sebastiencs#161, sebastiencs#168, sebastiencs#170.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@basil-conto basil-conto basil-conto left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

New changes in company-mode not compatible with company-box. This is making emacs unusable
5 participants
@CsBigDataHub @seagle0128 @sebastiencs @dgutov @basil-conto