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

sup-tweak-labels: sync back if specified #300

Closed

Conversation

kalbasit
Copy link
Contributor

RE: #295

sup-tweak will only sync back if

1 - :sync_back_to_maildir is set to true (in ~/.sup/config.yml)
2 - sup-tweak was ran with the option --sync-back
3 - The modified email comes from a source with sync_back enabled (in ~/.sup/sources.yml)

@gauteh
Copy link
Member

gauteh commented Apr 30, 2014

Thanks for the PR.

Actually; looking a bit more closely at the sync_back routines I saw that sync_back is guarded on each source anyway in Message::Location. Your code (and my original) will also not work quite as we expect since a Message will have many sources, m.source only gets the first location that is valid, so you are only checking the first valid source, while a call to m.sync_back will sync back to all the sources with sync_back enabled (message.rb:759).

Since a sync_back is guarded already for the global option (we can still keep the guard in -tweak-labels) and it is guarded per-source already, I think that the more appropriate solution is to rather have a --no-sync-back option - which will prevent any source from being sync'ed - whatever the global configuration and the per-source configuration.

@kalbasit
Copy link
Contributor Author

Those were my thoughts exactly, to add a --no-sync-back, However I thought that this might change the behavior of it and people will start experiencing an unexpected behavior. If you're up for it I'll change my code around to make it work.

@gauteh
Copy link
Member

gauteh commented Apr 30, 2014

Excerpts from Wael M. Nasreddine's message of 2014-04-30 20:01:17 +0200:

Those were my thoughts exactly, to add a --no-sync-back, However I thought that this might change the behavior of it and people will start experiencing an unexpected behavior. If you're up for it I'll change my code around to make it work.

that would be great! apparently not many people use tweak-back, it's
been broken for a while.

@kalbasit kalbasit changed the title sup-tweak: sync back if specified sup-tweak-labels: sync back if specified May 1, 2014
@kalbasit
Copy link
Contributor Author

kalbasit commented May 1, 2014

I changed it so the index never sync back and the message itself sync_back if it can.

gauteh pushed a commit that referenced this pull request May 1, 2014
Squashed commit of the following:

commit ebfb333
Author: Wael M. Nasreddine <[email protected]>
Date:   Wed Apr 30 22:26:10 2014 -0700

    sup-tweak-labels: add a newline before the begin

commit f1d51d3
Author: Wael M. Nasreddine <[email protected]>
Date:   Wed Apr 30 22:22:23 2014 -0700

    sup-tweak-labels: sync_back (honor the settings global and source) and provide an override

commit c43319a
Author: Wael M. Nasreddine <[email protected]>
Date:   Tue Apr 29 15:54:01 2014 -0700

    sup-tweak-labels: sync back if specified
@gauteh
Copy link
Member

gauteh commented May 1, 2014

Merged. Thanks.

@gauteh gauteh closed this May 1, 2014
@kalbasit kalbasit deleted the fix-sup-tweak-labels branch May 1, 2014 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants