Skip to content

Conversation

gjf7
Copy link

@gjf7 gjf7 commented Oct 10, 2024

Follow up #410, for #352.

"include": ["src", "**/api/Buffer.test.ts"],
"exclude": ["node_modules", "__tests__", "**/*.test.ts", "examples", "lib"]
"include": ["src"],
"exclude": ["node_modules", "examples", "lib"]
Copy link
Member

Choose a reason for hiding this comment

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

Nice, so we can close #352 ?

Copy link
Author

Choose a reason for hiding this comment

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

Not really, still need to work on packages/integration-tests/.

const buffer = await nvim.createBuffer(false, false);
expect(await nvim.buffers).toHaveLength(numBuffers + 1);

assert(buffer instanceof Buffer);
Copy link
Member

@justinmk justinmk Oct 10, 2024

Choose a reason for hiding this comment

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

Not sure if it's a typescript or tsserver bug, but locally I had to do this (similar for the other instanceof checks below):

Suggested change
assert(buffer instanceof Buffer);
assert(typeof buffer !== 'number');

The nvim.openWindow and related signatures are kind of weird.

Copy link
Author

Choose a reason for hiding this comment

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

It's ok on my mac and ci. Anyway, I changed this to assert(typeof buffer !== 'number').

Copy link
Author

Choose a reason for hiding this comment

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

The nvim.openWindow and related signatures are kind of weird.

Yeah, I think we can improve this in future.

*/
set tabpage(tabpage: AsyncTabpage) {
set tabpage(tabpage: AsyncTabpage | Tabpage) {
this.request(`${this.prefix}set_current_tabpage`, [tabpage]);
Copy link
Member

@justinmk justinmk Oct 10, 2024

Choose a reason for hiding this comment

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

I wonder if this and the buffer() case were a hidden bug. Do they need the same treatment as in window()? I don't see anywhere in this.request codepath that would implicitly handle a Promise (AsyncBuffer/AsyncTabpage)

    if (tabpage instanceof Tabpage) {
      this.request(`${this.prefix}set_current_tabpage`, [tabpage]);
    } else {
      tabpage.then(tabpage => this.request(`${this.prefix}set_current_tabpage`, [tabpage]));
    }

@justinmk
Copy link
Member

#411 (comment) doesn't need to block this, but might need a fix.

@justinmk justinmk merged commit a7d6c78 into neovim:master Oct 10, 2024
10 checks passed
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.

2 participants