[Musicpd-dev-team] [PATCH ncmpc] new key to edit lyric
Hi, There is a possibility to search for lyric on the Internet and to save it, but I miss a possibility to edit it, so I wrote it. Here it is. Jitka Novotna From 7d8252ae22a1b1b660a49ca584459b45adc4335b Mon Sep 17 00:00:00 2001 From: Jitka Novotna ji...@ucw.cz Date: Sun, 11 Dec 2011 12:50:02 +0100 Subject: [PATCH] new key to edit lyrics --- src/command.c |2 ++ src/command.h |1 + src/screen_help.c |1 + src/screen_lyrics.c | 32 4 files changed, 36 insertions(+), 0 deletions(-) diff --git a/src/command.c b/src/command.c index 0e7e5a2..43741cf 100644 --- a/src/command.c +++ b/src/command.c @@ -241,6 +241,8 @@ static command_definition_t cmds[] = { N_(Interrupt action) }, { {'u', 0, 0 }, 0, CMD_LYRICS_UPDATE, lyrics-update, N_(Update Lyrics) }, + { {'e', 0, 0 }, 0, CMD_LYRICS_EDIT, lyrics-edit, + N_(Edit Lyrics) }, #endif #ifdef ENABLE_OUTPUTS_SCREEN diff --git a/src/command.h b/src/command.h index d1e8122..a05ea55 100644 --- a/src/command.h +++ b/src/command.h @@ -96,6 +96,7 @@ typedef enum { CMD_SCREEN_LYRICS, CMD_SCREEN_OUTPUTS, CMD_LYRICS_UPDATE, + CMD_LYRICS_EDIT, CMD_INTERRUPT, CMD_GO_ROOT_DIRECTORY, CMD_GO_PARENT_DIRECTORY, diff --git a/src/screen_help.c b/src/screen_help.c index 038ab3b..e3b2f3d 100644 --- a/src/screen_help.c +++ b/src/screen_help.c @@ -162,6 +162,7 @@ static const struct help_text_row help_text[] = { from the server */ { 0, CMD_INTERRUPT, N_(Interrupt retrieval) }, { 0, CMD_LYRICS_UPDATE, N_(Download lyrics for currently playing song) }, + { 0, CMD_LYRICS_EDIT, N_(Add or edit lyrics) }, { 0, CMD_SAVE_PLAYLIST, N_(Save lyrics) }, { 0, CMD_DELETE, N_(Delete saved lyrics) }, #endif diff --git a/src/screen_lyrics.c b/src/screen_lyrics.c index 6de59d5..618623c 100644 --- a/src/screen_lyrics.c +++ b/src/screen_lyrics.c @@ -330,6 +330,35 @@ lyrics_paint(void) screen_text_paint(text); } +static void +lyric_edit(struct mpdclient *c) +{ + /* save current lyric to a file and run editor on it */ + pid_t cpid; + if ((cpid = fork()) != -1){ + if (cpid == 0){ + char * editor = getenv(EDITOR); + if (editor != NULL) { +ncu_deinit(); +if (!store_lyr_hd()){ + char path[1024]; + path_lyr_file(path, 1024, + current.artist, + current.title); + execl(editor,editor,path,0); +} + } + } else { + wait(cpid); + ncu_init(); + } + } + + /* lyrics update */ + screen_lyrics_load(c-song); + screen_text_repaint(text); +} + static bool lyrics_cmd(struct mpdclient *c, command_t cmd) { @@ -368,6 +397,9 @@ lyrics_cmd(struct mpdclient *c, command_t cmd) screen_text_repaint(text); } return true; + case CMD_LYRICS_EDIT: + lyric_edit(c); + return true; case CMD_SELECT: if (current.loader == NULL current.artist != NULL current.title != NULL) { -- 1.7.2.5 -- Learn Windows Azure Live! Tuesday, Dec 13, 2011 Microsoft is holding a special Learn Windows Azure training event for developers. It will provide a great way to learn Windows Azure and what it provides. You can attend the event by watching it streamed LIVE online. Learn more at http://p.sf.net/sfu/ms-windowsazure___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] [PATCH ncmpc] new key to edit lyric
On Mon, Dec 12, 2011 at 07:13:05PM +0100, Jitka Novotna wrote: Hi, There is a possibility to search for lyric on the Internet and to save it, but I miss a possibility to edit it, so I wrote it. Here it is. Jitka Novotna Thanks for the patch, I like it; but it has some problems. + { {'e', 0, 0 }, 0, CMD_LYRICS_EDIT, lyrics-edit, + N_(Edit Lyrics) }, It should probably rather just be called edit, as other screens might get an edit feature sooner or later. +static void +lyric_edit(struct mpdclient *c) I'd call it lyrics_edit (note the 's'). +{ + /* save current lyric to a file and run editor on it */ + pid_t cpid; + if ((cpid = fork()) != -1){ Perhaps we should use system() instead of fork and exec. Max? + if (cpid == 0){ + char * editor = getenv(EDITOR); + if (editor != NULL) { + ncu_deinit(); + if (!store_lyr_hd()){ + char path[1024]; + path_lyr_file(path, 1024, + current.artist, + current.title); + execl(editor,editor,path,0); execl only works with absolute paths. + } + } + } else { + wait(cpid); + ncu_init(); + } + } This code block could use some restructuring (which I'd be happy to do). + + /* lyrics update */ + screen_lyrics_load(c-song); + screen_text_repaint(text); This loads the wrong lyrics if you're viewing other lyrics than that of the current song. With the attached follow-up patch it compiles and works resonably, if I set $EDITOR (starting the right editor is _hard_. Debian has sensible- editor to do most of the tricky stuff, but you can't expect that everywhere). Again, thanks for the patch. It probably needs a bit of discussion (I'm just kind of a co-maintainer), but it seems useful. @Max: What about guarding this feature with #ifndef NCMPC_MINI, would that make sense? Regards, Jonathan Neuschäfer diff --git a/src/screen_lyrics.c b/src/screen_lyrics.c index 618623c..15a3e81 100644 --- a/src/screen_lyrics.c +++ b/src/screen_lyrics.c @@ -28,9 +28,11 @@ #include screen.h #include lyrics.h #include screen_text.h +#include ncu.h #include assert.h #include sys/stat.h +#include sys/wait.h #include stdlib.h #include string.h #include glib.h @@ -330,33 +332,37 @@ lyrics_paint(void) screen_text_paint(text); } +/* lyric_edit uses it */ +static bool lyrics_cmd(struct mpdclient *, command_t); + static void lyric_edit(struct mpdclient *c) { + ncu_deinit(); /* save current lyric to a file and run editor on it */ pid_t cpid; if ((cpid = fork()) != -1){ if (cpid == 0){ char * editor = getenv(EDITOR); if (editor != NULL) { -ncu_deinit(); if (!store_lyr_hd()){ char path[1024]; path_lyr_file(path, 1024, current.artist, current.title); - execl(editor,editor,path,0); + execlp(editor, editor, path, NULL); } } + /* getenv or exec failed, so exit */ + exit(EXIT_FAILURE); } else { - wait(cpid); - ncu_init(); + waitpid(cpid, NULL, 0); } } + ncu_init(); - /* lyrics update */ - screen_lyrics_load(c-song); - screen_text_repaint(text); + /* update to get the changes (this is a bit hacky) */ + lyrics_cmd(c, CMD_SELECT); } static bool -- Learn Windows Azure Live! Tuesday, Dec 13, 2011 Microsoft is holding a special Learn Windows Azure training event for developers. It will provide a great way to learn Windows Azure and what it provides. You can attend the event by watching it streamed LIVE online. Learn more at http://p.sf.net/sfu/ms-windowsazure___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] [PATCH ncmpc] new key to edit lyric
On Mon, Dec 12, 2011 at 1:39 PM, Jonathan Neuschäfer j.neuschae...@gmx.netwrote: With the attached follow-up patch it compiles and works resonably, if I set $EDITOR (starting the right editor is _hard_. Debian has sensible- editor to do most of the tricky stuff, but you can't expect that everywhere). It might be nice to look first at a (new) ncmpc config parameter, then EDITOR, then fall back to a default. As it is, if EDITOR is unset, a user might end up wondering why the edit key doesn't seem to do anything - and I think a lot of people don't have EDITOR set. Alternatively a helpful message could be printed, if we're worried about a user having no idea even how to quit vi(m). -- Learn Windows Azure Live! Tuesday, Dec 13, 2011 Microsoft is holding a special Learn Windows Azure training event for developers. It will provide a great way to learn Windows Azure and what it provides. You can attend the event by watching it streamed LIVE online. Learn more at http://p.sf.net/sfu/ms-windowsazure___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] [PATCH ncmpc] new key to edit lyric
On Mon, Dec 12, 2011 at 02:05:46PM -0800, Jeffrey Middleton wrote: On Mon, Dec 12, 2011 at 1:39 PM, Jonathan Neuschäfer j.neuschae...@gmx.netwrote: With the attached follow-up patch it compiles and works resonably, if I set $EDITOR (starting the right editor is _hard_. Debian has sensible- editor to do most of the tricky stuff, but you can't expect that everywhere). It might be nice to look first at a (new) ncmpc config parameter, then EDITOR, then fall back to a default. As it is, if EDITOR is unset, a user might end up wondering why the edit key doesn't seem to do anything - and I think a lot of people don't have EDITOR set. Alternatively a helpful message could be printed, if we're worried about a user having no idea even how to quit vi(m). Yes, I agree. A prompt asking whether to start an editor seems to be a good defalt action. So I propose two options: editor-ask = yes|no Ask before starting an editor on the lyrics screen editor = EDITOR The text editor for editing lyrics I dropped the lyrics- prefix, because some other screen/command might some day also want to spawn an editor (but I don't have a strong opinion on this). And maybe we should do the fall back mechanism (and reading EDITOR) it in a script, which would be the default editor. Thanks, Jonathan Neuschäfer -- Learn Windows Azure Live! Tuesday, Dec 13, 2011 Microsoft is holding a special Learn Windows Azure training event for developers. It will provide a great way to learn Windows Azure and what it provides. You can attend the event by watching it streamed LIVE online. Learn more at http://p.sf.net/sfu/ms-windowsazure ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] [PATCH ncmpc] new key to edit lyric
On 2011/12/12 22:39, Jonathan Neuschäfer j.neuschae...@gmx.net wrote: Perhaps we should use system() instead of fork and exec. Max? system() is easier to use and portable, but fork() gives you more control; for example, you could try a list of executables in one child process, instead of having to fork for every attempt. Portability is important, as Avuton is preparing a WIN32 build. If the feature is made in a way that is not portable, it should be auto-disabled (with #if) on incompatible platforms, or there should be a fallback. Max -- Learn Windows Azure Live! Tuesday, Dec 13, 2011 Microsoft is holding a special Learn Windows Azure training event for developers. It will provide a great way to learn Windows Azure and what it provides. You can attend the event by watching it streamed LIVE online. Learn more at http://p.sf.net/sfu/ms-windowsazure ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team