Sat 29 Mar 2014 12:52:43 AM UTC, comment #13:
Good, not just me...
|
Fri 28 Mar 2014 11:01:03 PM UTC, comment #12:
I missed a parenthesis. I checked in a changeset to fix that (http://hg.savannah.gnu.org/hgweb/octave/rev/077575fe9a7e).
|
Fri 28 Mar 2014 10:15:02 PM UTC, comment #11:
We have a build failure due to these changes, parse error near the printer error message in _print_parse_opts_.m:
http://hydra.nixos.org/build/9862648/log/tail-reload
|
Fri 28 Mar 2014 07:58:47 PM UTC, comment #10:
I had already gone ahead and committed the first part of the patch here (http://hg.savannah.gnu.org/hgweb/octave/rev/29f00c0d0657).
For the second part with Ghostscript devices I made this changeset (http://hg.savannah.gnu.org/hgweb/octave/rev/cdc16fbb513f).
|
Fri 28 Mar 2014 06:49:15 PM UTC, comment #9:
I have never seen a ghostscript device list that did not
contain ps2write at least so the additional test adds
little.
|
Fri 28 Mar 2014 03:30:55 PM UTC, comment #8:
I think there is still some confusion on the sense of the test. We are checking explicitly that the the printer device is one of the Ghostscript devices. If we want any other allowable format we need to put a test in the if conditional for it. Before, we had a test for postscript as well since this is a common enough format to send to the printer.
If modern printers can accept anything thrown at them then we should just eliminate this test completely and pass everything through.
|
Fri 28 Mar 2014 02:59:15 PM UTC, comment #7:
Yes, I think there was a parens problem, but my latest
version only tests for the device in the gnuplot
list, nothing else. Or, at least that is what I intended,
and what I think is appropriate. Using the pswrite or
ps2write restriction means that many printers cannot be
used.
So, just:
if (! (any (strcmp (arg_st.devopt, gs_device_list)))
error...
endif
should do it.
|
Fri 28 Mar 2014 02:02:50 PM UTC, comment #6:
I don't think so, look at the ordering of parentheses. The original patch you submitted prints an error if devopt is not in gs_device_list or if it is either pswrite or ps2write. The parentheses nesting is different in the test I posted, it errors if devopt is not in either gs_device_list or the list of pswrite and ps2write.
Maybe that's what you meant to change it to, but that's not what your patch did. That's how I read Rik's objection to your change also.
|
Fri 28 Mar 2014 01:50:28 PM UTC, comment #5:
Mike,
That was what the original patch did, but it does not really
make sense. My new suggested version just checks the gnuplot
known devices list. If that leads to a problem with a particular
printer it should be up to the user to figure out. Otherwise, we
start having to know what formats each printer in the world
will accept.
|
Fri 28 Mar 2014 01:44:27 PM UTC, comment #4:
Perhaps the logic was meant to be
This would be making it a more inclusive check, not more restrictive as it currently is. That seems to be what the comment before the error is implying.
|
Fri 28 Mar 2014 11:03:48 AM UTC, comment #3:
Rik,
The patch:
|| any (strcmp (arg_st.devopt, {"pswrite", "ps2write"}))
is obviously wrong. But, the original code is also
wrong. It does not allow spooling to a PDF printer or anything
except PS. Many modern printers accept other formats. In addition,
the "printer" may be a "virtual" printer which outputs PDF
rather than actually spooling to any device.
So, the only part of this test that is valid is the line
if (! any (strcmp (arg_st.devopt, gs_device_list))
This will allow formats that the specific printer does not
support but there is no obvious way of avoiding this.
So, the patch should read:
diff -r d0d9f6daa4b6 -r 90c199446cea scripts/plot/util/private/__print_parse_opts__.m
--- a/scripts/plot/util/private/__print_parse_opts__.m Mon Mar 24 21:24:16 2014 -0700
+++ b/scripts/plot/util/private/__print_parse_opts__.m Tue Mar 25 17:00:41 2014 +0000
@@ -335,7 +335,8 @@
error ("print: a file name may not specified when spooling to a printer")
endif
if (! any (strcmp (arg_st.devopt, gs_device_list))
- || ! any (strcmp (arg_st.devopt, {"pswrite", "ps2write"})))
+ )
- ## Only postscript and supported ghostscript devices
- error ("print: invalid format for spooling to a printer")
+ error ("print: invalid format for ghostscript")
endif
OK?
|
Thu 27 Mar 2014 03:21:51 PM UTC, comment #2:
>> ishandle([])
ans =
[]
>> version
ans =
7.9.0.529 (R2009b)
About, the change to
- || ! any (strcmp (arg_st.devopt, {"pswrite", "ps2write"})))
I thought I had a reason for that, but please remove it until
I have a chance to rethink.
|
Thu 27 Mar 2014 03:12:43 PM UTC, comment #1:
Could you run the following test code in Matlab?
Does this return an empty matrix, or 0 because an empty matrix is definitely not a handle?
In part of the patch you have
The intent was to check for formats which couldn't be spooled to the printer. On UNIX systems postscript usually can be sent to the printer which is why there was a check for the format NOT being pswrite. By dropping the '!' we will now error out for what seems a legitimate case.
|
Tue 25 Mar 2014 05:19:42 PM UTC, original submission:
This patch corrects 2 problems:
1. print -Pa_printer introduces an extraneous -P so that
the resulting lpr command fails. Removed -P.
2. The test for the presence of a figure at the beginning of
print() fails to detect the lack of a figure. This may
be due to a change in isfigure(). If there is no figure
and the opts.figure value is set to empty, isfigure returns
true. Use of isempty instead of isfigure fixes this.
In addition I edited the print help info. Currently it was
unclear and referred to the ability to generate TeX (as
opposed to LaTeX. Since it unconditionally gereates LaTeX
I removed the TeX references and tried to clarify the operation
of the latex modes.
The print command is quite complex and handles lots of different
cases and both fltk and gnuplot. So, it would be helpful if
these patches could be tested in other environments. So far
they work for me and make generating plots with real TeX in the
text strings much more convenient.
|