patch for issue 708

classic Classic list List threaded Threaded
23 messages Options
12
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

patch for issue 708

Andrew Hawryluk
This patch will allow convert-ly to process this:

\version "2.11.0"

{
c d'4 ees
\set Staff.keySignature = #`(((1 . 4) . 2) ((1 . 3) . 2) ((3 . 3) 2))

f^"some text"
\set Staff.keySignature = #`(((1 . 4) . -2)
                             ((1 . 3) . -4))
}


and output this:

convert-ly (GNU LilyPond) 2.13.1
Processing `test.ly'...
Applying conversion: 2.11.2, 2.11.3, 2.11.5, 2.11.6, 2.11.10, 2.11.11,
2.11.13, 2.11.15, 2.11.23, 2.11.35, 2.11.38, 2.11.46, 2.11.48,
2.11.50, 2.11.51, 2.11.52, 2.11.53, 2.11.55, 2.11.57, 2.11.60,
2.11.61, 2.11.62, 2.11.64, 2.12.0, 2.12.3, 2.13.0,
Not smart enough to convert Staff.keySignature - the alist is no
longer in reversed order.
2.13.1
\version "2.13.1"

{
c d'4 ees
\set Staff.keySignature = #`(((1 . 4) . ,SHARP)
                             ((1 . 3) . ,SHARP)
                             ((3 . 3) . ,SHARP))

f^"some text"
\set Staff.keySignature = #`(((1 . 4) . ,FLAT)
                             ((1 . 3) . ,DOUBLE-FLAT))
}

0002-Improved-keySignature-support-in-convert-ly.patch (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: patch for issue 708

Carl Sorensen
Thanks,  Applied.

Valentin,

Can you change the status of 708 to fixed in 2.13.1?  And verify, at your
convenience?

Thanks,

Carl


On 5/22/09 9:11 PM, "Andrew Hawryluk" <[hidden email]> wrote:

> This patch will allow convert-ly to process this:
>
> \version "2.11.0"
>
> {
> c d'4 ees
> \set Staff.keySignature = #`(((1 . 4) . 2) ((1 . 3) . 2) ((3 . 3) 2))
>
> f^"some text"
> \set Staff.keySignature = #`(((1 . 4) . -2)
>                              ((1 . 3) . -4))
> }
>
>
> and output this:
>
> convert-ly (GNU LilyPond) 2.13.1
> Processing `test.ly'...
> Applying conversion: 2.11.2, 2.11.3, 2.11.5, 2.11.6, 2.11.10, 2.11.11,
> 2.11.13, 2.11.15, 2.11.23, 2.11.35, 2.11.38, 2.11.46, 2.11.48,
> 2.11.50, 2.11.51, 2.11.52, 2.11.53, 2.11.55, 2.11.57, 2.11.60,
> 2.11.61, 2.11.62, 2.11.64, 2.12.0, 2.12.3, 2.13.0,
> Not smart enough to convert Staff.keySignature - the alist is no
> longer in reversed order.
> 2.13.1
> \version "2.13.1"
>
> {
> c d'4 ees
> \set Staff.keySignature = #`(((1 . 4) . ,SHARP)
>                              ((1 . 3) . ,SHARP)
>                              ((3 . 3) . ,SHARP))
>
> f^"some text"
> \set Staff.keySignature = #`(((1 . 4) . ,FLAT)
>                              ((1 . 3) . ,DOUBLE-FLAT))
> }


---

----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: patch for issue 708

Graham Percival
On Sat, May 23, 2009 at 05:20:56PM -0600, Carl D. Sorensen wrote:
> Can you change the status of 708 to fixed in 2.13.1?  And verify, at your
> convenience?

Actually, the idea is that the programmer (or comitter) would
change the status to fixed, and Valentin would verify it when
2.13.1 GUB is released.

For 708, I've changed the status to fixed.

Cheers,
- Graham

---

----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: patch for issue 708

Patrick McCarty
On Sat, May 23, 2009 at 7:26 PM, Graham Percival
<[hidden email]> wrote:
> On Sat, May 23, 2009 at 05:20:56PM -0600, Carl D. Sorensen wrote:
>> Can you change the status of 708 to fixed in 2.13.1?  And verify, at your
>> convenience?
>
> Actually, the idea is that the programmer (or comitter) would
> change the status to fixed, and Valentin would verify it when
> 2.13.1 GUB is released.
>
> For 708, I've changed the status to fixed.

I don't see the patch on git master.  Did you push the patch, Carl?

-Patrick

---

----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: patch for issue 708

Carl Sorensen



On 5/23/09 8:39 PM, "Patrick McCarty" <[hidden email]> wrote:

> On Sat, May 23, 2009 at 7:26 PM, Graham Percival
> <[hidden email]> wrote:
>> On Sat, May 23, 2009 at 05:20:56PM -0600, Carl D. Sorensen wrote:
>>> Can you change the status of 708 to fixed in 2.13.1?  And verify, at your
>>> convenience?
>>
>> Actually, the idea is that the programmer (or comitter) would
>> change the status to fixed, and Valentin would verify it when
>> 2.13.1 GUB is released.
>>
>> For 708, I've changed the status to fixed.
>
> I don't see the patch on git master.  Did you push the patch, Carl?

I did, but I had a network problem so it didn't finish.  It's there now.

Thanks,

Carl

>
> -Patrick
>
> ---
>
> ----
> Join the Frogs!
>


---

----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: patch for issue 708

Carl Sorensen
In reply to this post by Graham Percival



On 5/23/09 8:26 PM, "Graham Percival" <[hidden email]> wrote:

> On Sat, May 23, 2009 at 05:20:56PM -0600, Carl D. Sorensen wrote:
>> Can you change the status of 708 to fixed in 2.13.1?  And verify, at your
>> convenience?
>
> Actually, the idea is that the programmer (or comitter) would
> change the status to fixed, and Valentin would verify it when
> 2.13.1 GUB is released.

OK.  I thought that only Valentin changed the status.

If committers are allowed to change the status, I'll do that in the future.

>
> For 708, I've changed the status to fixed.

Thanks, Graham!

Carl


---

----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: patch for issue 708

Neil Puttock
In reply to this post by Carl Sorensen
2009/5/24 Carl D. Sorensen <[hidden email]>:
> Thanks,  Applied.

Unfortunately, there are two serious flaws here:

- keySignature alists which aren't backquoted (e.g., the example in
the bug tracker) will be ignored

- entries of the form (notename . alteration) are mangled:

\set Staff.keySignature = #'((0 . 2) (1 . 2)  (4 . 2))

-> \set Staff.keySignature = #`(((0 . 2) . ,SEMI-SHARP)
                             ((2 . 4) . ,SHARP))

Less seriously, the two conversion functions appear to be identical
apart from the different dictionaries for alterations.  Would it be
possible to use a single `fixKS' function with the dictionaries passed
as an argument to cut down on the duplication?

Regards,
Neil

---

----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: patch for issue 708

Carl Sorensen



On 5/24/09 4:49 AM, "Neil Puttock" <[hidden email]> wrote:

> 2009/5/24 Carl D. Sorensen <[hidden email]>:
>> Thanks,  Applied.
>
> Unfortunately, there are two serious flaws here:
>
> - keySignature alists which aren't backquoted (e.g., the example in
> the bug tracker) will be ignored
>
> - entries of the form (notename . alteration) are mangled:
>
> \set Staff.keySignature = #'((0 . 2) (1 . 2)  (4 . 2))
>
> -> \set Staff.keySignature = #`(((0 . 2) . ,SEMI-SHARP)
>                              ((2 . 4) . ,SHARP))
>
> Less seriously, the two conversion functions appear to be identical
> apart from the different dictionaries for alterations.  Would it be
> possible to use a single `fixKS' function with the dictionaries passed
> as an argument to cut down on the duplication?


Thanks for catching this, Neil.

Andrew, I've reverted the patch.  Could you rewrite it to fix these issues?

Graham, I added a comment to the bugtracker, and tried to change the status,
but I couldn't find a way to do it?  Do I have access to change status?

Neil,

On a more general note, do you have any suggestions for how to check
convert-ly rules?  For code, we have regression tests.  For convert-ly, as
far as I know, we have nothing.  Should we be establishing convert-ly
regression tests?

Thanks,

Carl


---

----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: patch for issue 708

Graham Percival
On Sun, May 24, 2009 at 07:14:28AM -0600, Carl D. Sorensen wrote:
>
> Graham, I added a comment to the bugtracker, and tried to change the status,
> but I couldn't find a way to do it?  Do I have access to change status?

You have the same access as Valentin, and he can do this stuff.
To change the status, you need "add a comment" (even if you don't
want to leave a comment), then select the Status drop-down box.

> On a more general note, do you have any suggestions for how to check
> convert-ly rules?  For code, we have regression tests.  For convert-ly, as
> far as I know, we have nothing.  Should we be establishing convert-ly
> regression tests?

I hope think that we won't have so many convert-ly rules that this
become necessary.  Certainly at the moment I think it would take
more work than it would be worth.

If we ever get a dedicated convert-ly person (another unfilled job
for years *sigh*), then this could be a good idea.

Cheers,
- Graham

---

----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: patch for issue 708

Carl Sorensen



On 5/24/09 7:25 AM, "Graham Percival" <[hidden email]> wrote:

> On Sun, May 24, 2009 at 07:14:28AM -0600, Carl D. Sorensen wrote:
>>
>> Graham, I added a comment to the bugtracker, and tried to change the status,
>> but I couldn't find a way to do it?  Do I have access to change status?
>
> You have the same access as Valentin, and he can do this stuff.
> To change the status, you need "add a comment" (even if you don't
> want to leave a comment), then select the Status drop-down box.

Maybe it's because it's a closed issue, but when I selected "add a comment",
there was now Status drop-down box, at least that I could see.

Carl


---

----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: patch for issue 708

Graham Percival
On Sun, May 24, 2009 at 07:29:37AM -0600, Carl D. Sorensen wrote:

>
> On 5/24/09 7:25 AM, "Graham Percival" <[hidden email]> wrote:
>
> > On Sun, May 24, 2009 at 07:14:28AM -0600, Carl D. Sorensen wrote:
> >>
> >> Graham, I added a comment to the bugtracker, and tried to change the status,
> >> but I couldn't find a way to do it?  Do I have access to change status?
> >
> > You have the same access as Valentin, and he can do this stuff.
> > To change the status, you need "add a comment" (even if you don't
> > want to leave a comment), then select the Status drop-down box.
>
> Maybe it's because it's a closed issue, but when I selected "add a comment",
> there was now Status drop-down box, at least that I could see.

Are you /sure/?  That's odd, because we've had non-developers
screwing with the status.

Like, go to issue 708.  See that big box for "add a comment and
make changes"?  click in that box.

A bunch of options now open up beneath it.  Summary, Status,
Owner, etc.  Click on that "fixed" beside the "Status" title.
It's supposed to come up with a menu.


What step doesn't work?  Oh, and are you logged into google with
your google id?  Maybe it doesn't give you one of those options if
you're not logged in...?

Cheers,
- Graham

---

----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: patch for issue 708

Valentin Villenave
Administrator
2009/5/24 Graham Percival <[hidden email]>:
> Are you /sure/?  That's odd, because we've had non-developers
> screwing with the status.

I'm not sure they have. They may open irrelevant issues, post useless
comments, but I don't think they have the ability to change status.

> Like, go to issue 708.  See that big box for "add a comment and
> make changes"?  click in that box.
>
> A bunch of options now open up beneath it.  Summary, Status,
> Owner, etc.  Click on that "fixed" beside the "Status" title.
> It's supposed to come up with a menu.

You are also supposed to type "fixed_2_13_2" in a blank input box. Or
whatever the next dev version will be numbered.

Regards,
Valentin

---

----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: patch for issue 708

Patrick McCarty
On Sun, May 24, 2009 at 8:59 AM, Valentin Villenave
<[hidden email]> wrote:
> 2009/5/24 Graham Percival <[hidden email]>:
>> Are you /sure/?  That's odd, because we've had non-developers
>> screwing with the status.
>
> I'm not sure they have. They may open irrelevant issues, post useless
> comments, but I don't think they have the ability to change status.

When I'm logged into my Google account, I only see an "Add a comment"
box.  Clicking inside the box doesn't provide any more options besides
"Attach a file".

-Patrick

---

----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: patch for issue 708

Patrick McCarty
On Sun, May 24, 2009 at 11:14 AM, Patrick McCarty <[hidden email]> wrote:

> On Sun, May 24, 2009 at 8:59 AM, Valentin Villenave
> <[hidden email]> wrote:
>> 2009/5/24 Graham Percival <[hidden email]>:
>>> Are you /sure/?  That's odd, because we've had non-developers
>>> screwing with the status.
>>
>> I'm not sure they have. They may open irrelevant issues, post useless
>> comments, but I don't think they have the ability to change status.
>
> When I'm logged into my Google account, I only see an "Add a comment"
> box.  Clicking inside the box doesn't provide any more options besides
> "Attach a file".

Maybe only the owners and members listed on this page have permission
to change issue status, etc.:

http://code.google.com/p/lilypond/

I don't recall Neil ever changing an issue's status, even though he is
a developer...

-Patrick

---

----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: patch for issue 708

Valentin Villenave
Administrator
2009/5/24 Patrick McCarty <[hidden email]>:

> Maybe only the owners and members listed on this page have permission
> to change issue status, etc.:

Yes, I believe Graham was wrong on this one :-)

Regards,
Valentin

---

----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: patch for issue 708

Carl Sorensen
In reply to this post by Graham Percival



On 5/24/09 7:42 AM, "Graham Percival" <[hidden email]> wrote:

> On Sun, May 24, 2009 at 07:29:37AM -0600, Carl D. Sorensen wrote:
>>
>> On 5/24/09 7:25 AM, "Graham Percival" <[hidden email]> wrote:
>>
>>> On Sun, May 24, 2009 at 07:14:28AM -0600, Carl D. Sorensen wrote:
>>>>
>>>> Graham, I added a comment to the bugtracker, and tried to change the
>>>> status,
>>>> but I couldn't find a way to do it?  Do I have access to change status?
>>>
>>> You have the same access as Valentin, and he can do this stuff.
>>> To change the status, you need "add a comment" (even if you don't
>>> want to leave a comment), then select the Status drop-down box.
>>
>> Maybe it's because it's a closed issue, but when I selected "add a comment",
>> there was now Status drop-down box, at least that I could see.
>
> Are you /sure/?  That's odd, because we've had non-developers
> screwing with the status.
>
> Like, go to issue 708.  See that big box for "add a comment and
> make changes"?  click in that box.
>
> A bunch of options now open up beneath it.  Summary, Status,
> Owner, etc.  Click on that "fixed" beside the "Status" title.
> It's supposed to come up with a menu.
>
>
> What step doesn't work?  Oh, and are you logged into google with
> your google id?  Maybe it doesn't give you one of those options if
> you're not logged in...?
Yes, I'm logged in.

What I see on the screen is attached in a .png.

Thanks,

Carl


IssueComment.png (38K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: patch for issue 708

Andrew Hawryluk
In reply to this post by Carl Sorensen
On Sun, May 24, 2009 at 7:14 AM, Carl D. Sorensen <[hidden email]> wrote:

>
>
>
> On 5/24/09 4:49 AM, "Neil Puttock" <[hidden email]> wrote:
>
>> 2009/5/24 Carl D. Sorensen <[hidden email]>:
>>> Thanks,  Applied.
>>
>> Unfortunately, there are two serious flaws here:
>>
>> - keySignature alists which aren't backquoted (e.g., the example in
>> the bug tracker) will be ignored
>>
>> - entries of the form (notename . alteration) are mangled:
>>
>> \set Staff.keySignature = #'((0 . 2) (1 . 2)  (4 . 2))
>>
>> -> \set Staff.keySignature = #`(((0 . 2) . ,SEMI-SHARP)
>>                              ((2 . 4) . ,SHARP))
>>
>> Less seriously, the two conversion functions appear to be identical
>> apart from the different dictionaries for alterations.  Would it be
>> possible to use a single `fixKS' function with the dictionaries passed
>> as an argument to cut down on the duplication?
>
>
> Thanks for catching this, Neil.
>
> Andrew, I've reverted the patch.  Could you rewrite it to fix these issues?


Yes, I'll take a look at it. Thanks, Neil for catching those!
Won't be this week, but I'll keep you posted.

Andrew

---

----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: patch for issue 708

Graham Percival
In reply to this post by Carl Sorensen
On Sun, May 24, 2009 at 06:23:07PM -0600, Carl D. Sorensen wrote:
>
> On 5/24/09 7:42 AM, "Graham Percival" <[hidden email]> wrote:
>
> > Are you /sure/?  That's odd, because we've had non-developers
> > screwing with the status.
>
> Yes, I'm logged in.
>
> What I see on the screen is attached in a .png.

Ok, I'm a big man[1].  I can admit that I was wrong.  I guess that
the guess about only administrators and submitters being able to
modify the status was correct.
... but I still /do/ recall Joe adding labels like fixed_2_11_25.
Hmm, I'll need to look into this a bit more.  Maybe google code
changed recently?  Joe's not a project administrator.

[1] well, bigger than I was in early Jan.  Eating a hamburger
almost every day does that to you...


Wait a moment -- Valentin just changed the status, and he's not an
admin!
... aha!  Carl, you weren't a "project member".  I've changed
this, so now you should see the magical other options.  :)

Cheers,
- Graham

---

----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: patch for issue 708

Carl Sorensen



On 5/25/09 3:42 AM, "Graham Percival" <[hidden email]> wrote:

>
> Wait a moment -- Valentin just changed the status, and he's not an
> admin!
> ... aha!  Carl, you weren't a "project member".  I've changed
> this, so now you should see the magical other options.  :)

Darn, now I can't pawn my work off on Graham....

Yes, the other options are there now.

Thanks, Graham.

Carl


---

----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: patch for issue 708

Neil Puttock
In reply to this post by Andrew Hawryluk
2009/5/25 Andrew Hawryluk <[hidden email]>:

> Yes, I'll take a look at it. Thanks, Neil for catching those!

No problem.

I'm sorry I didn't respond earlier; though I'd taken a cursory look at
the patch (and noticed the backquote issue), I didn't expect it to be
committed so soon.

Regards,
Neil

---

----
Join the Frogs!

12
Loading...