regression: point-and-click no longer includes absolute path

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

regression: point-and-click no longer includes absolute path

Mark Knoop-4
The commit below has broken point-and-click links for me on Linux.
Embedded paths are now relative rather than absolute. It seems the
assumption in the commit message that an absolute path is always
returned is incorrect, as can be seen with the following example.

Reverting scm/output-ps.scm fixes the problem.

\version "2.19.40"
testpaths =
#(define-void-function
   (parser location)
   ()
   (display (car (ly:input-file-line-char-column location))))
\testpaths


commit f30a8189adbbeefa2103e2c2e194040f66bc2291
Refs: release/2.19.27-1-280-gf30a818
Author:     Urs Liska <[hidden email]>
AuthorDate: Tue Jan 19 10:52:33 2016 +0100
Commit:     Urs Liska <[hidden email]>
CommitDate: Sun Jan 24 21:24:52 2016 +0100

    #4747: Remove (all) uses of is-absolute?

    The check for absolute paths in  in output-ps.scm
    and -svg.scm is unnecessary because
    (car ly:input-file-line-char-column a-location)
    always returns an absolute, slashified path

    Now is-absolute? is not used anymore by LilyPond itself.


--
Mark Knoop

_______________________________________________
bug-lilypond mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-lilypond
Reply | Threaded
Open this post in threaded view
|

Re: regression: point-and-click no longer includes absolute path

Urs Liska
Which version of LilyPond and what Linux exactly do you use (2.19.40
binary release?)?

Compiling your example with LilyPond built from current master gives me:

Starting lilypond 2.19.42 [Untitled]...
Processing `/tmp/frescobaldi-dW8ldh/tmpTuF3xf/document.ly'
Parsing...
/tmp/frescobaldi-dW8ldh/tmpTuF3xf/document.ly
Success: compilation successfully completed


(and that what was the case when I made that change (and obviously noone
had objections then)).

So please show me what you get in return from that function on your
system. Or an example where point-and-click links are actually generated
relatively and break anything.

Urs

Am 13.05.2016 um 09:16 schrieb Mark Knoop:

> The commit below has broken point-and-click links for me on Linux.
> Embedded paths are now relative rather than absolute. It seems the
> assumption in the commit message that an absolute path is always
> returned is incorrect, as can be seen with the following example.
>
> Reverting scm/output-ps.scm fixes the problem.at
>
> \version "2.19.40"
> testpaths =
> #(define-void-function
>    (parser location)
>    ()
>    (display (car (ly:input-file-line-char-column location))))
> \testpaths
>
>
> commit f30a8189adbbeefa2103e2c2e194040f66bc2291
> Refs: release/2.19.27-1-280-gf30a818
> Author:     Urs Liska <[hidden email]>
> AuthorDate: Tue Jan 19 10:52:33 2016 +0100
> Commit:     Urs Liska <[hidden email]>
> CommitDate: Sun Jan 24 21:24:52 2016 +0100
>
>     #4747: Remove (all) uses of is-absolute?
>
>     The check for absolute paths in  in output-ps.scm
>     and -svg.scm is unnecessary because
>     (car ly:input-file-line-char-column a-location)
>     always returns an absolute, slashified path
>
>     Now is-absolute? is not used anymore by LilyPond itself.
>
>

_______________________________________________
bug-lilypond mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-lilypond
Reply | Threaded
Open this post in threaded view
|

Re: regression: point-and-click no longer includes absolute path

Mark Knoop-4
Thanks Urs,

At 09:37 on 13 May 2016, Urs Liska wrote:
>Which version of LilyPond and what Linux exactly do you use (2.19.40
>binary release?)?

lilypond 2.19.40, on Fedora 23, distro build (lilypond-2.19.40-1.fc23)

However I have just downloaded 2.19.41 binary release from lilypond.org
and see the same behavior (I added { c } to the code below to
generate a pdf):

GNU LilyPond 2.19.41
Processing `testpaths.ly'
Parsing...testpaths.ly
Interpreting music...
Preprocessing graphical objects...
Finding the ideal number of pages...
Fitting music on 1 page...
Drawing systems...
Layout output to `/tmp/lilypond-DUHuav'...
Converting to `testpaths.pdf'...
Deleting `/tmp/lilypond-DUHuav'...
Success: compilation successfully completed

The point-and-click link in the resulting pdf is relative:
textedit://testpaths.ly:9:2:3 - and therefore doesn't work.

>Compiling your example with LilyPond built from current master gives
>me:
>
>Starting lilypond 2.19.42 [Untitled]...
>Processing `/tmp/frescobaldi-dW8ldh/tmpTuF3xf/document.ly'
>Parsing...
>/tmp/frescobaldi-dW8ldh/tmpTuF3xf/document.ly
>Success: compilation successfully completed

Hmm, that's interesting. Is this perhaps because you are running via
frescobaldi which passes the full pathname of the file to lilypond? Can
you try running directly with a relative path.

ly:input-file-line-char-column seems to return the path which is given
on the commandline, whether it is relative or absolute.

>(and that what was the case when I made that change (and obviously
>noone had objections then)).
>
>So please show me what you get in return from that function on your
>system. Or an example where point-and-click links are actually
>generated relatively and break anything.
>
>Urs
>
>Am 13.05.2016 um 09:16 schrieb Mark Knoop:
>> The commit below has broken point-and-click links for me on Linux.
>> Embedded paths are now relative rather than absolute. It seems the
>> assumption in the commit message that an absolute path is always
>> returned is incorrect, as can be seen with the following example.
>>
>> Reverting scm/output-ps.scm fixes the problem.
>>
>> \version "2.19.40"
>> testpaths =
>> #(define-void-function
>>    (parser location)
>>    ()
>>    (display (car (ly:input-file-line-char-column location))))
>> \testpaths
>>
>>
>> commit f30a8189adbbeefa2103e2c2e194040f66bc2291
>> Refs: release/2.19.27-1-280-gf30a818
>> Author:     Urs Liska <[hidden email]>
>> AuthorDate: Tue Jan 19 10:52:33 2016 +0100
>> Commit:     Urs Liska <[hidden email]>
>> CommitDate: Sun Jan 24 21:24:52 2016 +0100
>>
>>     #4747: Remove (all) uses of is-absolute?
>>
>>     The check for absolute paths in  in output-ps.scm
>>     and -svg.scm is unnecessary because
>>     (car ly:input-file-line-char-column a-location)
>>     always returns an absolute, slashified path
>>
>>     Now is-absolute? is not used anymore by LilyPond itself.
>>
--
Mark Knoop

_______________________________________________
bug-lilypond mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-lilypond

testpaths.ly (170 bytes) Download Attachment
testpaths.pdf (35K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: regression: point-and-click no longer includes absolute path

Urs Liska


Am 17.05.2016 um 09:31 schrieb Mark Knoop:
> Thanks Urs,
>
> At 09:37 on 13 May 2016, Urs Liska wrote:
>> Which version of LilyPond and what Linux exactly do you use (2.19.40
>> binary release?)?
> lilypond 2.19.40, on Fedora 23, distro build (lilypond-2.19.40-1.fc23)
>
> However I have just downloaded 2.19.41 binary release from lilypond.org

OK, given the stuff below that doesn't matter anymore:

> and see the same behavior (I added { c } to the code below to
> generate a pdf):
>
> GNU LilyPond 2.19.41
> Processing `testpaths.ly'
> Parsing...testpaths.ly
> Interpreting music...
> Preprocessing graphical objects...
> Finding the ideal number of pages...
> Fitting music on 1 page...
> Drawing systems...
> Layout output to `/tmp/lilypond-DUHuav'...
> Converting to `testpaths.pdf'...
> Deleting `/tmp/lilypond-DUHuav'...
> Success: compilation successfully completed
>
> The point-and-click link in the resulting pdf is relative:
> textedit://testpaths.ly:9:2:3 - and therefore doesn't work.

Hm, the PDF you attached *does* give an absolute link, but:

>
>> Compiling your example with LilyPond built from current master gives
>> me:
>>
>> Starting lilypond 2.19.42 [Untitled]...
>> Processing `/tmp/frescobaldi-dW8ldh/tmpTuF3xf/document.ly'
>> Parsing...
>> /tmp/frescobaldi-dW8ldh/tmpTuF3xf/document.ly
>> Success: compilation successfully completed
> Hmm, that's interesting. Is this perhaps because you are running via
> frescobaldi which passes the full pathname of the file to lilypond? Can
> you try running directly with a relative path.
>
> ly:input-file-line-char-column seems to return the path which is given
> on the commandline, whether it is relative or absolute.

Indeed, this is right, so obviously my patch *did* change the behaviour
of point-and-click. (I have to repeat that nobody noticed that during
review).

Please don't reply to this post as I'll open a new thread to discuss
whether this is a bug or a feature ;-)

Best
Urs

>
>> (and that what was the case when I made that change (and obviously
>> noone had objections then)).
>>
>> So please show me what you get in return from that function on your
>> system. Or an example where point-and-click links are actually
>> generated relatively and break anything.
>>
>> Urs
>>
>> Am 13.05.2016 um 09:16 schrieb Mark Knoop:
>>> The commit below has broken point-and-click links for me on Linux.
>>> Embedded paths are now relative rather than absolute. It seems the
>>> assumption in the commit message that an absolute path is always
>>> returned is incorrect, as can be seen with the following example.
>>>
>>> Reverting scm/output-ps.scm fixes the problem.
>>>
>>> \version "2.19.40"
>>> testpaths =
>>> #(define-void-function
>>>    (parser location)
>>>    ()
>>>    (display (car (ly:input-file-line-char-column location))))
>>> \testpaths
>>>
>>>
>>> commit f30a8189adbbeefa2103e2c2e194040f66bc2291
>>> Refs: release/2.19.27-1-280-gf30a818
>>> Author:     Urs Liska <[hidden email]>
>>> AuthorDate: Tue Jan 19 10:52:33 2016 +0100
>>> Commit:     Urs Liska <[hidden email]>
>>> CommitDate: Sun Jan 24 21:24:52 2016 +0100
>>>
>>>     #4747: Remove (all) uses of is-absolute?
>>>
>>>     The check for absolute paths in  in output-ps.scm
>>>     and -svg.scm is unnecessary because
>>>     (car ly:input-file-line-char-column a-location)
>>>     always returns an absolute, slashified path
>>>
>>>     Now is-absolute? is not used anymore by LilyPond itself.
>>>
>
>
> _______________________________________________
> bug-lilypond mailing list
> [hidden email]
> https://lists.gnu.org/mailman/listinfo/bug-lilypond

_______________________________________________
bug-lilypond mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-lilypond
Reply | Threaded
Open this post in threaded view
|

Re: regression: point-and-click no longer includes absolute path

Mark Knoop-4
>> The point-and-click link in the resulting pdf is relative:
>> textedit://testpaths.ly:9:2:3 - and therefore doesn't work.
>
>Hm, the PDF you attached *does* give an absolute link, but:

Yes, sorry, testing with different versions and attached the wrong one.
See attached pdf now.

>Indeed, this is right, so obviously my patch *did* change the behaviour
>of point-and-click. (I have to repeat that nobody noticed that during
>review).

It could be that most point-and-click users are also users of
Frescobaldi or other front-ends that pass the full pathname to lilypond.

--
Mark Knoop

_______________________________________________
bug-lilypond mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-lilypond

testpaths.pdf (34K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: regression: point-and-click no longer includes absolute path

David Kastrup
Mark Knoop <[hidden email]> writes:

>>> The point-and-click link in the resulting pdf is relative:
>>> textedit://testpaths.ly:9:2:3 - and therefore doesn't work.
>>
>>Hm, the PDF you attached *does* give an absolute link, but:
>
> Yes, sorry, testing with different versions and attached the wrong one.
> See attached pdf now.
>
>>Indeed, this is right, so obviously my patch *did* change the
>>behaviour of point-and-click. (I have to repeat that nobody noticed
>>that during review).
>
> It could be that most point-and-click users are also users of
> Frescobaldi or other front-ends that pass the full pathname to
> lilypond.

I would think that often potential reviewers entertain the naive notion
that a patch submitter has tested the most basic functionality of his
patches as stated in the issue description.

So the reviewers do not even try what is declared as the basic idea of
the patch (that is, if they try anything at all).

But realistically, we don't get a whole lot of pre-acceptance testing
anyway.  So it's always a good idea to check out stuff oneself,
optimally also submitting regtests that are able to certify the change
in behavior.

Yes, this happens less often than desirable.  But "I have to repeat that
nobody noticed that during review" is a bit silly, given how few of our
developers indulge in full-scale manual reviews and testing.  Our
automated tests (performed and evaluated using manual labour, though)
_do_ catch quite a few things.  But if they are supposed to demonstrate
the validity of a change, there must be a regtest created for the
change.

--
David Kastrup

_______________________________________________
bug-lilypond mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-lilypond