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

Simplify multi-column to single column , fixes #4184 #4837

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jeffgoh
Copy link

@jeffgoh jeffgoh commented Mar 9, 2024

Fixes #4184 - Negative repeat count does nothing at /usr/lib64/perl5/vendor_perl/Cyrus/IMAP/Shell.pm line 688

Fixes - Negative repeat count does nothing at /usr/lib64/perl5/vendor_perl/Cyrus/IMAP/Shell.pm line 688
Simplify multi-column to single column fixes cyrusimap#4184
Copy link
Contributor

@elliefm elliefm left a comment

Choose a reason for hiding this comment

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

Thanks for submitting this! I've been re-reading the original issue to refresh my memory, and in a comment there, I said this:

I think all the calculation of $w, $ll, $n, $c are unnecessary once the column formatting is discarded, so you should also remove the parts that calculate those values. $l and @l look like they're still needed though.

You haven't removed those parts though, do you mind doing so (and test it out)? I could do it myself, but I don't actually use cyradm, so it would be hard for me to check whether I've accidentally broken something. I think the changes you need are like this:

  • line 662 -> remove $w,
  • line 668 -> remove line (no longer need to initialise $w)
  • lines 677-679 -> remove lines (no longer need to compute $w)
  • lines 684-687 -> remove lines (no longer need $ll, $w, $n, $c)

You might want to do these in the reverse order so the line numbers don't change as you make the changes... :)

Also, this PR branch has a merge commit on it. Could you please rebase to get rid of the merge, and then force-push? If you don't know how to do this, that's okay, I'll land the PR manually and do it myself in the process. But if you do, it would make life easier. Thanks!

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.

cyradm perl warnings on perl v5.34.1
2 participants