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

[WIP] Port window-scroll-bars #1538

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[WIP] Port window-scroll-bars #1538

wants to merge 1 commit into from

Conversation

clample
Copy link
Contributor

@clample clample commented Aug 14, 2019

This PR ports window-scroll-bars. I still need to add some ert tests, though.

@clample clample changed the title Port window-scroll-bars [WIP] Port window-scroll-bars Aug 14, 2019
} else {
None
};
list!(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did the original use Fcons instead of list6()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe that they only have up to list5:

remacs/src/lisp.h

Lines 3706 to 3711 in fb2e5d7

extern Lisp_Object list1 (Lisp_Object);
extern Lisp_Object list2 (Lisp_Object, Lisp_Object);
extern Lisp_Object list3 (Lisp_Object, Lisp_Object, Lisp_Object);
extern Lisp_Object list4 (Lisp_Object, Lisp_Object, Lisp_Object, Lisp_Object);
extern Lisp_Object list5 (Lisp_Object, Lisp_Object, Lisp_Object, Lisp_Object,
Lisp_Object);

With list!, I think the rust version really shines

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed. list! is so much nicer. I suspected that was the answer about list5, but I did not have the chance to check.

@shaleh
Copy link
Collaborator

shaleh commented Jan 12, 2020

What is left to complete for this PR?

@clample
Copy link
Contributor Author

clample commented Jan 13, 2020

@shaleh sorry for abandoning this one for a while. If I remember correctly, I wanted to merge #1535 first as it had some tests using window scroll bars that I thought would be nice to have additional coverage.

That PR had some issues I was unable to figure out, though.

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