[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
