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

sc-im: aligning a selection of cells cause crash #335133

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

sc-im: aligning a selection of cells cause crash #335133

wants to merge 1 commit into from

Conversation

mipmip
Copy link

@mipmip mipmip commented Aug 16, 2024 •
edited
Loading

Description of changes

By disabling fortify in gcc this fixes #335130 sc-im: aligning a selection of cell's cause crash

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@mipmip mipmip mentioned this pull request Aug 16, 2024
Open
@NixOSInfra NixOSInfra added the 12. first-time contribution This PR is the author's first one; please be gentle! label Aug 16, 2024
@ofborg ofborg bot requested a review from dotlambda August 16, 2024 15:44
@dotlambda
Copy link
Member

Not true. The commit message should start with sc-im.

@smancill
Copy link
Contributor

Please edit the commit summary to sc-im: fix crash and put more details in the body.

@mipmip mipmip changed the title fix #335130 sc-im: aligning a selection of cell's cause crash sc-im: aligning a selection of cells cause crash Aug 20, 2024
@mipmip
Copy link
Author

mipmip commented Aug 20, 2024

Please edit the commit summary to sc-im: fix crash and put more details in the body.

I just did. Sorry for my beginner mistakes.

@ofborg ofborg bot requested a review from dotlambda August 20, 2024 16:47
@smancill
Copy link
Contributor

smancill commented Aug 20, 2024 •
edited
Loading

Looks OK to me but I cannot reproduce the bug on Darwin. Needs someone with Linux to test it:

  • Move with hjkl
  • Insert text with \<text><enter>
  • Repeat for a few cells
  • Start visual selection with v and select all cells
  • Justify with { or }
  • Quit with :q

@smancill
Copy link
Contributor

Result of nixpkgs-review pr 335133 run on x86_64-darwin 1

1 package built:
  • sc-im

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 9, 2024
@korrat
Copy link
Contributor

korrat commented Nov 17, 2024

I was able to verify the commit on x86_64-linux.

On recent nixpkgs (9g23szywla53rajzivcisxkiv415hwy0-sc-im-0.8.4), sc-im dumps core when trying to align the cells. With this PR (x6psi9sw54xyq2aa34k0p3ss1pxb8c4i-sc-im-0.8.4), aligning works as expected.

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 335133

x86_64-linux

✅ 1 package built:
  • sc-im

@smancill
Copy link
Contributor

@mipmip please rebase to handle merge conflicts.

@mipmip
sc-im: fix aligning selection of cells cause crash
55a2b3e 
Fixes NixOS#335130

For complete description of problem with video see
andmarti1424/sc-im#884
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 19, 2024
@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FliegendeWurst FliegendeWurst FliegendeWurst approved these changes

@dotlambda dotlambda Awaiting requested review from dotlambda

Assignees
No one assigned
Labels
10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 10.rebuild-linux: 1 12.approvals: 1 This PR was reviewed and approved by one reputable person 12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

sc-im: aligning a selection of cell's cause crash
7 participants
@mipmip @dotlambda @smancill @korrat @FliegendeWurst @wegank @NixOSInfra