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

Experimental/newsreader #5

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

Experimental/newsreader #5

wants to merge 4 commits into from

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Sep 21, 2012

No description provided.

@oli-obk
Copy link
Contributor Author

oli-obk commented Nov 26, 2013

updated

meebey
meebey reviewed Nov 26, 2013
View reviewed changes
src/Common/RSSFeed.cs
/*
* Copyright (c) 2008 Carlos Martín Nieto
*
* This file is released under the terms of the GNU GPLv2.
Copy link
Owner

Choose a reason for hiding this comment

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

@carlosmn GPLv2 only is a problem for us, we need v3 compat. Carlos, do you agree to relicense this to "GPLv2 or later"?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, let's make it v2+.

@meebey
Copy link
Owner

meebey commented Nov 26, 2013

Please rebase on master which now includes a single MessageBuilder.NormalizeNewlines() implemention.

@oli-obk
Copy link
Contributor Author

oli-obk commented Dec 27, 2013

rebased & stuffs

@oli-obk
Copy link
Contributor Author

oli-obk commented Dec 27, 2013

oh wait
forgot the gpl version

@oli-obk
Copy link
Contributor Author

oli-obk commented Dec 27, 2013

like so?

@oli-obk
Copy link
Contributor Author

oli-obk commented Jan 21, 2014

ping?

meebey
meebey reviewed Jan 21, 2014
View reviewed changes
src/Engine-Feed/Protocols/News/FeedProtocolManager.cs
foreach (var feed in _Feeds) {
builder = CreateMessageBuilder();
builder.AppendText(feed.Url.AbsoluteUri);
command.FrontendManager.AddMessageToChat(command.Chat, builder.ToMessage());
Copy link
Owner

Choose a reason for hiding this comment

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

command.FrontendManager must not be used directly (since hooks), instead use Session.AddMessageToFrontend(). Applies to all use.

@meebey
Copy link
Owner

meebey commented Jan 21, 2014

General remarks:

  • rebase, it no longer applies cleanly in master.
  • I haven't checked in detail which features FeedProtocolManager has but I suspect it has less than what my version in Session did. This checking this is a show stopper as you replaced it.
  • make coverage, I doubt this passes a make call
  • It is probably a good idea to support throwing arbitrary feeds into a chat, you should look how I implemented streaming in Twitter here (I am fine though if this is postponed for later): eb64cb2#diff-11f722a95fdf4291f9485bc6a051eb30R28

@oli-obk
Copy link
Contributor Author

oli-obk commented Jan 21, 2014

*rebased
*the only "missing" "feature" is that the date of the entry is not displayed behind the title, but rather the normal smuxi timestamps are used
*i am already throwing arbitrary feeds into a protocolchat, the issue is that i'm polling, and not receiving a stream -> i need to poll all feeds that are in the same chat at the same time so the timestamp order doesn't go haywire

@oli-obk
Copy link
Contributor Author

oli-obk commented Jan 21, 2014

i'll have a screenshot for you in a minute

@oli-obk
Copy link
Contributor Author

oli-obk commented Jan 21, 2014

screenshot from 2014-01-21 12 37 20
screenshot from 2014-01-21 12 37 17

@meebey meebey added 0.12 and removed 0.11 labels Apr 4, 2014
@meebey meebey modified the milestones: 0.12, 0.11 Apr 4, 2014
@oli-obk
Copy link
Contributor Author

oli-obk commented Sep 13, 2017

Yea I don't think this is happening anymore...

@oli-obk oli-obk closed this Sep 13, 2017
@meebey
Copy link
Owner

meebey commented Nov 4, 2017

This is a lovely feature that I want to see and use in Smuxi, thus re-opening the PR.

@meebey meebey reopened this Nov 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
0.12
Projects
None yet
Milestone
0.12
Development

Successfully merging this pull request may close these issues.
3 participants
@oli-obk @meebey @carlosmn