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

Support for tweet-threads and related changes #64

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

Conversation

twoscomplement
Copy link
Collaborator

For consideration/discussion wrt issue #23

  • Omit the horizontal rule between tweets where one is a reply to the previous tweet
  • Add an anchor to each tweet
  • Remove the "Replying to" header - instead, link @ names to the tweet (on twitter) being replied to
  • When outputting replies to self that don't immediately follow the tweet being replied to, print an up arrow ↑ that links to the anchor of the previous tweet in the thread
  • When outputting a tweet that has replies that don't immediately follow, print down arrows ↓ that link to the anchor of each reply

(I'm not sure of the method of linking is actually useful - id's and filenames are valid, but may need to be expressed differently to work as intended in practice)

- Omit the horizontal rule between tweets where one is a reply to the previous tweet
- Add an anchor to each tweet
- Remove the "Replying to" header - instead, link @NAMEs to the tweet (on twitter) being replied to
- When outputting replies to self that don't immediately follow the tweet being replied to, print an up arrow ↑ that links to the anchor of the previous tweet in the thread
- When outputting a tweet that has replies that don't immediately follow, print down arrows ↓ that link to the anchor of each reply

(I'm not sure of the method of linking is actually useful - id's and filenames are valid, but may need to be expressed differently to work as intended in practice)
md_string += '\n\n----\n' if md_string else ''
if(t.reply_to_self):
# a reply-to-self, but not a reply to the most recently output tweet: add a header
md_string += f'\n[↑]({tweet_files[t.reply_to_id]}#s{t.reply_to_id})' if t.reply_to_id in tweet_files else '↑'
Copy link
Owner

Choose a reason for hiding this comment

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

This breaks the new html output, sadly, since it links to an md file.

Copy link
Owner

@timhutton timhutton Nov 17, 2022

Choose a reason for hiding this comment

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

@twoscomplement I'm thinking of rewriting the HTML output to be inside parser.py, produced alongside the markdown. This would make fixing this issue trivial. HTML is now produced alongside the MD.

@timhutton
Copy link
Owner

@twoscomplement The links mostly seem to work in the md files, when viewed in Visual Studio Code at least. They take you to the right file but don't necessarily scroll down to the right place.

@timhutton
Copy link
Owner

I like that it doesn't change the order of the tweets.

@twoscomplement
Copy link
Collaborator Author

I'll look into reworking this once #67 is merged - might take me a few days

@lenaschimmel lenaschimmel changed the title Support for threads and related changes Support for tweet-threads and related changes Nov 28, 2022
@lenaschimmel
Copy link
Collaborator

Hi @twoscomplement, are you still interested in finishing this PR? PR #67 is merged now. Is there anything we can do to assist? Or would you like someone else to take over your PR and finish it? Either way it's fine.

(I also changed the title slightly, so it's not confused with multi-threaded implementation)

@twoscomplement
Copy link
Collaborator Author

Hi @lenaschimmel - thanks for asking, I've been short on time to revisit this.

If there's someone else interested in picking this up, I'd be happy for them to do so - and happy to to cheer and/or review the work 🙂

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.

3 participants