Skip to content

bpo-6532: Make the thread id an unsigned integer. #781

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

Merged
merged 2 commits into from
Mar 23, 2017

Conversation

serhiy-storchaka
Copy link
Member

From C API side the type of results of PyThread_start_new_thread() and
PyThread_get_thread_ident(), the id parameter of
PyThreadState_SetAsyncExc(), and the thread_id field of PyThreadState
changed from "long" to "unsigned long".

From C API side the type of results of PyThread_start_new_thread() and
PyThread_get_thread_ident(), the id parameter of
PyThreadState_SetAsyncExc(), and the thread_id field of PyThreadState
changed from "long" to "unsigned long".
@serhiy-storchaka serhiy-storchaka added the type-feature A feature request or enhancement label Mar 23, 2017
@mention-bot
Copy link

@serhiy-storchaka, thanks for your PR! By analyzing the history of the files in this pull request, we identified @Birne94, @warsaw and @Yhg1s to be potential reviewers.

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM except of a minor issue.

return NULL;
}
return PyLong_FromLong(ident);
unsigned long ident = PyThread_get_thread_ident();
Copy link
Member

Choose a reason for hiding this comment

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

In _PyImport_ReleaseLock(), you test "== PYTHREAD_INVALID_THREAD_ID". Please be consistent: remove the test in _PyImport_ReleaseLock(), or restore the test it. I'm in favor of dropping the test: pthread_self() and GetCurrentThreadId() cannot fail.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch. Don't know why I removed the check here. The code was initially written 4 years ago.

@vstinner vstinner merged commit aefa7eb into python:master Mar 23, 2017
@vstinner
Copy link
Member

Thanks for your contrib Serhiy! LGTM.

@serhiy-storchaka serhiy-storchaka deleted the thread-id-unsigned branch March 25, 2017 10:18
andersk added a commit to andersk/zulip that referenced this pull request Oct 19, 2022
timabbott pushed a commit to zulip/zulip that referenced this pull request Oct 19, 2022
amandaalexandre pushed a commit to amandaalexandre/zulip that referenced this pull request Oct 25, 2022
narrow: Update browser title when in a narrow search view.

Sets a default value of "Search results" for complicated narrow search
views and updates logic to use `filter.get_title` as a helper to
generate better titles for some common search views.

Does not update the existing behavior for narrow searches that have
"pm-with" or "group-pm-with" operators.

Note as of this change, the default search title and titles generated
from `filter.get_title` will be translated into the user's preferred
language.

Fixes zulip#22952.

compose: Stop selecting compose box content after topic change.

Previously, the on_topic_change handler for the compose system would
focus the compose box after a topic change, and also select all content in
the compose box. Selecting that content makes sense if we think the user's
intent is likely to be deleting that content; but there's no clear reason we
should expect that intent, and it's not particularly consistent with our drafts
model to risk accidentally losing partially composed message content this way.

After some discussion, we've concluded that in both cases reaching this code path
(either the previous topic being "" or the compose box content being entry), it'd
be better to skip the `.select()`.

Fixes zulip#23146.

recent_topics: Handle page up/down manually.

Fixes zulip#23078.

Instead of relying on browser, we handle the page up/down keys
to ensure we take care of some rows being hidden due to compose
box and table header.

migrations: Remove bugged Recipients in 0419.

right_sidebar: Fix full name overflowing its container.

User's name with status emoji was overflowing its container,
resulting in overflown text to not be converted into ellipsis
and pushing status emoji out of visible area.

realm_user_settings: Fix user status with emoji preview.

Fixed to reflect how user status with emoji will be displayed
in the right sidebar.

use_info_popover: Fix long words overflowing out of container.

The text will now break at the overflowing points. A hyphen maybe
inserted but is dependent on language and browser support.

help: Fix "Reply mentioning user" documentation.

Corrects "Reply mentioning user" option which is available in the
user menu if accessed from a message.

Documents "Copy mention syntax" option which is available from the
right sidebar.

Fixes: zulip#23202.

help center: Clean up documentation on sending messages.

- Consolidate /help/starting-a-new-private-thread and /help/private-messages.
- Tweak the wording on compose-and-send-message.md.
- Use compose-and-send-message.md where appropriate (and not otherwise).

css: Fix color of filter buttons being changed on hover for spectators.

We don't want the color to change at all on hover since these buttons are
disabled. This only needed a fix for the dark theme.

help: Rename "Add custom profile fields" -> "Custom profile fields".

Renames the help article on custom profile fields to reflect that
its content is not just about adding fields.

Adds a redirect from the old URL to the new URL and updates internal
links, linking to #add-a-custom-profile-field where appropriate.

Fixes zulip#23170.

help: Add "Edit a custom profile field" section.

Adds a section on editing to the Custom profile fields help article.

Fixes part of zulip#23170.

portico: Change deafult width of dropdowns to fit-content.

We seem to override this setting for most of the dropdowns, so this
common CSS only applies to the smaller dropdown shown to logged-in
users, where it was weirdly wide.

message_edit: Remove NO_LONGER from editability_types.

Previously, NO_LONGER type was just used to display
the text in the bottom-right of message edit form
which we have removed now, so we can remove `NO_LONGER`
type now.

hotkey: Use "m" hotkey for moving messages.

Fixes zulip#23113.

popovers: Add "(e)" shortcut to "View message source" option.

keyboard_shortcuts: Fix description for "e" shortcut.

This commit updates the description for "e" shortcut so
that it at least fits in one line in english.

Fixes zulip#23112.

popovers: Use can_move_message function.

We can now directly use `can_move_message` function instead
of checking topic and stream edit permission. This helps us
in avoid duplication of code.

message_edit: Allow editing "no topic" message from modal in all cases.

Previously, we allowed editing topic of "no topic" message, when other
conditions were not met, only from recipient bar but it is allowed
irrespective of other condtions. This commit fixes it to show the
"Move message" option and icon for "no topic" messages irrespective
of other conditions.

popovers: Rename "Move message" option to "Move messages".

This commit renames "Move message" option to "Move messages"
to make it more clear that the user can move multiple messages.

integrations: Increase font size of logo disclaimer.

Increased font size from 11px to 13px. This text was unreadable
on mobile as per Google report at 11px font size.

timeout: Correct thread id type passed to PyThreadState_SetAsyncExc.

This type changed in Python 3.7:
python/cpython#781

Signed-off-by: Anders Kaseorg <[email protected]>

recent_topics_ui: Use topics_widget to get number of current rows.

I think this should be more reliable and faster.

recent_topics: Check if at last row before moving down.

This looks a regression from some of the recent additions to
recent topics.

recent_topics: Set focus to first/last row on page up/down at ends.

This change is to match the behaviour of page up/down on messages.
So, If you hit PageUp/Down while there's no additional pace to scroll,
we move the selected topic to the first/last topic as appropriate.

unread: Fix recent topics exception when marking as unread.

This simple regression was introduced in
550a32b.

recent_topics: Add mention indicator in row for unread topics.

Fixes zulip#22984

Add an `@` icon in unread topics where user is mentioned.

We track a new set of `stream_id:topic` pairs for the unread mentions
so that recent topics instantly knows if a topic is unread and mentioned
or not.

docs: Update Zulip version mentioned and remove extraneous comma.

email_validation: Restore case-insensitive domain validation.

This was broken by commit b945aa3
(zulip#22604), because email_to_domain implicitly lowercased the result.

No adjustment is needed for is_disposable_domain, which already
lowercases its argument.

Signed-off-by: Anders Kaseorg <[email protected]>

stream_settings: Remove extraneous heading from message retention section

user_info_popover: Fix overflow of bot owner list item.

tippy: Wrap overflowing content of tippy tooltips.

settings: Reorganize organization settings sidebar

Fixes: zulip#23233

user_profile: Move title and tab switcher to a fixed header.

While going through the full profile of a user, we expect to find
that specific user's in-depth details. Thus, we should have a fixed
title displaying the name at the top of the modal to establish the
connection of the profile data with the user.

We also fix the tab switcher between "Profile/Streams/User groups" to
the top of the modal, for the added convinience of switching across
the three options at any point of time.

user_profile: Remove stale no-fields CSS class.

Previously, we had some special CSS for the user profile modals in the
case that there were no custom profile fields, to do things like hide
the <hr> divider between the normal profile fields and custom ones.

The design has for some time treated custom profile fields as just
additional profile fields; so there's no longer a need for custom CSS.

documentation_crawler: Exclude communities page.

Since /communities/#all was failing the spider test due to `#all`
not being present as an `id` on the page, we exclude it from
this check saying that it is not really a documentation page.

docs: Remove link to defunct GIF Brewery website.

portico: Add navigation links for Open communities directory.

help center: Update "Communities directory" help page.

This update reflects the fact that the page is now live.

portico: Link to Open communities directory from /for/X pages.

stream-create: Use simplebar.

This changes the stream creation form so that it uses simplebar
instead of the native scroll bar.

resolves zulip#21307

navbar: Add login and signup buttons for spectators.

Remove login buttons from right sidebar and add them to navbar
so that the user instantly notices them.

recent-topics: Rename to "Recent conversations" in web-app and docs.

Replaces instances of "recent topics" in the web-app and documentation
to be "recent conversations".

Renames both `recent-topics.md` files in the help center to be
`recent-conversations.md` and updates/redirects links to new URL.

Does not update instances of "recent topics" in frontend code comments
and does not update the main overview changelog, for now.

Does not change case study text where "recent topics" was referenced
in a quote, but does change generic text references to be "recent
conversations".

changelog: Make references to "Recent topics" consistent.

Updates the current 6.0 release notes to include information about
the rename to "Recent conversations", and updates past references
to "recent topics" to be consistently formatted as "Recent topics".

message_feed: Fix /me message alignment of message actions.

These were apparently no longer aligned with the timestamp.

Fixes zulip#23101.

message_feed: Fix alignment of EDITED notices in /me message.

Fixes part of zulip#23101.

message_edit: Fix vertical alignment of input area in /me messages.

Fixes zulip#23101.

drafts: Fix topic update logic handling of incomplete drafts.

This fixes a couple significant issues with the drafts logic:
* Previously, we would move drafts even when !going_forward_change; this
  is inconsistent with the compose box logic on the previous line.
* The drafts.rename_topic() function did not handle drafts with
  incomplete stream/topic fields correctly, throwing an exception in
  that case; this resulted in user-visible live update problems
  because code after the drafts.rename_topic call would not be
  executed.

message_edit: Remove unnecessary comment.

This comment is not required now since topic and
stream editing is not allowed from this UI now
and is instead done from a modal.

message_edit: Extract function to check whether message is valid.

We do not allow sending messages where sending request had failed
or message is locally echoed. This commit extracts these checks
to a new function so that we can avoid duplicating code.

ui_init: Fix condition in message_hover.

The message_hover function in ui_init.js had an if condition
to return early when is_topic_editable was false. We instead
change it to return early when message.sent_by_me is false
since in that case the edit_content element is not present.
Note that we also show the "View source" icon when message
is locally echoed or not sent successfully.

Till now is_topic_editable is always true when message is sent
by the user itself, and when message is sent by someone else
the edit_content element does not exist in the DOM anyway. So,
basically, it just helps in returing early and not unnecessarily
move forward to set html for a non-existent div. So, we can
ideally replace it with just `sent_by_me` condition and otherwise
we already show the reaction icon.

The comment above that also mentioned that "locally echoed messages
have !is_topic_editable" which is not true since is_topic_editable
does not check whether the message is locally-echoed or not. This
comment was added in e1c4c7b.

This also fixes the bug where "View source" icon is not shown when
"message.sent_by_me" is true and both editing and moving message
is not allowed which currently happens when allow_message_editing
setting is False.

Also, the "is_topic_editable" condition will be not valid in future
when "is_topic_editable" can be false when "sent_by_me" is true. In
such cases we would not show any icon then, so better to fix this
here.

Some history of this code-
- Intially we returned only when !sent_by_me was false, but added
a condition to return early when realm_allow_community_topic_editing
was false in 91197fa. Though it was not needed since edit_content
element amyways did not existed when `sent_by_me` was false.
- Then the condition was then changed to return early if
is_topic_editable was false in 689c717.

message_edit: Test is_topic_editable separately.

We now do not test is_topic_editable function in
get_editability test for better readability and
this will be helpful when we will need to test
more cases as part of zulip#21739.

message_edit: Do not allow moving locally echoed or failed messages.

We should not show UI for moving locally echoed or failed messages.

For the edit/move icon in message controls -
- After 2451002, we showed "Move message" icon for locally echoed
messages when moving messages was allowed as per org permissions.
This was clearly a bug and this commit fixes it and we instead
show "View source" icon.

For "m" hotkey -
- After 2451002, pressing "m" opened "Move messages" modal for
locally echoed messages when moving messages was allowed as per
org permissions. This was clearly a bug and this commit fixes it
and pressing "m" does nothing.

For pencil icon in recipient header -
- Previously, the pencil icon was showed even if first message
below the recipient bar was locally echoed and the topic edit
failed silently with locally echoed or failed messages again
showing under old topic after reload. This commit fixes it
to hide the icon in such cases. The original behavior was
that server returned an error with not so appropriate
response.
- Note that we can only check the first message below the header
since it will be inefficient to check all the messages whether
each of them is successfully sent.
And in such cases when any of the later messages is locally
echoed, the topic edit succeeds but the locally echoed or failed
messages are shown under old topic after reload.
- The behavior is same for the "checkmark" icon for resolving
topics in the header.

For three-dot message menu -
- We don't need to do anything here since the three-dot icon
is not shown for locally echoed or failed messages.

For the "Move topic" option in topic popover in left sidebar -
- We cannot do anything here since it will be inefficient
to check all messages in the topic whether they are locally
echoed or not everytime we open the popover.
- If the first message in topic is locally echoed or a failed
message, then the id of first message of the topic returned
from server is undefined and we see an error in client while
trying to get message from that id. And if one or multiple of
the later messages is locally echoed, then only successful and
server-acked messages are moved.

Note that this cannot handle all cases like a user can use
a successfully sent message to access the topic edit UI
and then choose "change_all" and "change_later" propagate
modes. In such cases the locally echoed and failed messages
will remain in the old topic only.

message_edit: Add comment for currently_echoing_messages check.

realm_domains: Populate the table only if the modal is open.

Fixes zulip#23315.

test_classes: Improve assert_database_query_count output message.

Output message should talk about both the cases:
actual_count > expected_count and actual_count < expected_count.

The message now includes information for the case where
actual_query_count < expected_query_count.

Fixes: zulip#23325

recent_topics_ui: Correct column number of read and mute row.

unread: Rename function name for clarity.

Renamed get_msg_ids_for_person to get_msg_ids_for_user_ids_string.

unread: Rename function name for clarity.

Renamed num_unread_for_person to num_unread_for_user_ids_string.

recent_topics_ui: Extend check for unread to PMs.

Since we also have PMs now, `has_unread` should consider that case
too. This seems like a good change regardless.

unread_ops: Add function to mark PM threads as read.

This will be used recent topics to mark PM rows as read.

recent_topics_ui: Allow unread count click in PM row to mark row as read.

Fixes zulip#23261

We now send user to next column after marking a row read. Added a
bit of bottom padding below unread count indicator.

message feed: Show the 3-dots icon while actions popover is open.

This commit enables the 3-dot menu to be visible while the associated
actions popover is open using the fact that the 'has_actions_popover'
class is attached to the message with an open popover. This approach
solves the bug without any additional JavaScript code.

Fixes part of zulip#23157

left-sidebar: Show the 3-dots icon when the menu is selected.

This commit fixes the issue where the 3-dots menu icons on the left
sidebar disappear even though the associated popover is open by
toggling a special CSS class on the menu item. It follows the same
approach used by the emoji picker on the message feed UI.

Fixes  zulip#23157

message feed UI: Remove redundant color declaration.

The color for the message control buttons is set in the
`message_control_button` class. The `reaction_button_visible` class
also sets a color declaration with the same value.

This commit removes this declaration because it doesn't change the
color of the reaction icon(the class is currently only used in
conjunction with `message_control_button` to toggle the button
visibility) nor does it override any inherited  style (hover color
remains the same regardless) as it lacks an `!important` rule.

popovers: Avoid focus outline on \vdots when actions popover is open.

Now that we show the \vdots icon while the actions popover is open,
there's a weird looking behavior where a focus outline appears around
the \vdots icon, only if you clicked on the icon itself, not its
square containing click arrow.

Disable this unintended behavior, which disrupts the otherwise
simulated state where focus is in the popover (but no row is focused).

style: adding flex display to message container

css: align the wrapped text in the serach message feed box to the right of the I icon.
alexmv pushed a commit to alexmv/zulip that referenced this pull request Nov 14, 2022
This type changed in Python 3.7:
python/cpython#781

Signed-off-by: Anders Kaseorg <[email protected]>
(cherry picked from commit 89e4233)
timabbott pushed a commit to zulip/zulip that referenced this pull request Nov 15, 2022
This type changed in Python 3.7:
python/cpython#781

Signed-off-by: Anders Kaseorg <[email protected]>
(cherry picked from commit 89e4233)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants