Skip to content
This repository has been archived by the owner on Oct 10, 2019. It is now read-only.

Regression after #593 #597

Open
Minoru opened this issue Sep 3, 2017 · 2 comments
Open

Regression after #593 #597

Minoru opened this issue Sep 3, 2017 · 2 comments
Labels

Comments

@Minoru
Copy link
Collaborator

Minoru commented Sep 3, 2017

@epon93 (epon on IRC) reports that the following macro sometimes errors with "Browser failed to open the link":

macro m set browser "mpv %u </dev/null &>/dev/null &"; open-in-browser-and-mark-read ; set browser "firefox %u"

Removing the ampersand after the command didn't help.

@epon93
Copy link

epon93 commented Sep 4, 2017

Here are some debug logs if it helps:

[2017-09-03 22:53:25] DEBUG: view::run: running macro `m'
[2017-09-03 22:53:25] DEBUG: formaction::process_op: running commandline `set "browser" "mpv %u </dev/null &>/dev/null &" '
[2017-09-03 22:53:25] DEBUG: configcontainer::set_configvalue(browser [resolved: browser],mpv %u </dev/null &>/dev/null &) called

[2017-09-03 22:53:25] INFO: itemlist_formaction: opening item at pos `2'
[2017-09-03 22:53:25] DEBUG: view::open_in_browser: running `mpv 'https://www.youtube.com/watch?v=redacted' </dev/null &>/dev/null &'
[2017-09-03 22:53:25] DEBUG: view::open_in_browser: couldn't create a child process
[2017-09-03 22:53:25] DEBUG: formaction::process_op: running commandline `set "browser" "firefox %u" '
[2017-09-03 22:53:25] DEBUG: configcontainer::set_configvalue(browser [resolved: browser],firefox %u) called

Note that mpv does actually get called properly and the video plays, but newsbeuter still throws the error and does not follow up with marking the item as read.

@Minoru Minoru removed the to-check label Sep 4, 2017
@Minoru
Copy link
Collaborator Author

Minoru commented Sep 4, 2017

I ran a bisect and the bug first showed up in e0f9100.

Even if I trim the command in the macro down to "mpv %u", it still sometimes fail, which totally baffles me.

This is a regression, so I'm reverting the changes for now. We'll have to figure out what's going on here, then bring the commits back (luckily they're still in our history).

I'd like to explicitly state that @Arlon1 did nothing wrong—if anything, his code simply uncovered a bug we've had for years.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants