-
Notifications
You must be signed in to change notification settings - Fork 25
Update for InkScape 1.0 #27
base: master
Are you sure you want to change the base?
Conversation
<id>id.giac.export.jpg</id> | ||
<dependency type="executable" location="extensions">jpegexport.py</dependency> | ||
<param name="path" type="string" _gui-text="Export path" _gui-description="Full path to your file, e.g. 'C:\Users\User_Name\Documents\myimage.jpg'"></param> | ||
<id>fablabchemnitz.de.jpegexport</id> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi - why did you change id and the submenu where the extension appears? Is this helping users?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry no. i did it change in my own desktop package because i have a lot of extensions and marked all other extension than default with fablab to have better overview of what i put next to them. for this repo we should keep as is/was
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:) Thanks!
self.OptionParser.add_option("--density", action="store", type="int", dest="density", default="90", help="") | ||
self.OptionParser.add_option("--page", action="store", type="inkbool", dest="page", default=False, help="") | ||
self.OptionParser.add_option("--fast", action="store", type="inkbool", dest="fast", default=True, help="") | ||
self.arg_parser.add_argument("--path", action="store", type=str, dest="path", default="", help="") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
action="store" and type=str can be removed. A help text would be quite useful, though. It will appear when the extension is used from the command line.
self.OptionParser.add_option("--fast", action="store", type="inkbool", dest="fast", default=True, help="") | ||
self.arg_parser.add_argument("--path", action="store", type=str, dest="path", default="", help="") | ||
self.arg_parser.add_argument("--bgcol", action="store", type=str, dest="bgcol", default="#ffffff", help="") | ||
self.arg_parser.add_argument("--quality", action="store", type=int, dest="quality", default="90", help="") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the int values, a min and max should be indicated. The default values for int entries are something like 0 to 10 (or something similarly useless).
self.arg_parser.add_argument("--quality", action="store", type=int, dest="quality", default="90", help="") | ||
self.arg_parser.add_argument("--density", action="store", type=int, dest="density", default="90", help="") | ||
self.arg_parser.add_argument("--page", action="store", type=inkex.Boolean, dest="page", default=False, help="") | ||
self.arg_parser.add_argument("--fast", action="store", type=inkex.Boolean, dest="fast", default=True, help="") | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dest="" can be removed, too, from all of them, it's not needed.
Find an example here:
https://gitlab.com/Moini/inkscape-guide-tools/-/blob/master/src/guide_creation_tools.py#L51
@@ -72,7 +73,7 @@ def effect(self): | |||
bgcol = self.options.bgcol | |||
|
|||
if self.options.page == False: | |||
if len(self.selected) == 0: | |||
if len(self.svg.selected) == 0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is going to be deprecated again in the next version. Just a heads up - will work with 1.0 and 1.1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey. what will be the replacement in newer InkScape? Can you give some point for this? I am working hard to migrate all my used plugins to python 3 where nobody else did it yet. i dont want to do this twice per year :-D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's a list of currently known deprecations for 1.1:
https://gitlab.com/inkscape/extensions/-/commit/4838d285b01106d243399e6bf36e7956a291eb98
There's also the bounding box thing among them (but it's for the current selection, so maybe not what you need).
else: # uses command line | ||
rawprops=[] | ||
for prop in props: | ||
command=("inkscape", "--without-gui", "--query-id", id, "--query-"+prop, self.args[-1]) | ||
command=("inkscape", "--without-gui", "--query-id", id, "--query-"+prop, self.options.input_file) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this actually work? The without-gui option no longer exists.
h = -bbox[2] + bbox[3] | ||
starty = toty - math.ceil(bbox[2]) -h | ||
endy = toty - math.ceil(bbox[2]) | ||
bbox = sum([node.bounding_box() for node in nodelist], None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't there a function for this in inkex already?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm i updated this using the provided tip from deprecation warning. did not find some shorter line for this
command="inkscape -a %s:%s:%s:%s -d %s -e \"%sjpinkexp.png\" -b \"%s\" %s" % (x0, y0, x1, y1, self.options.density, tmp, bgcol, curfile) | ||
|
||
p = subprocess.Popen(command, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) | ||
command="inkscape --export-area %s:%s:%s:%s -d %s --export-filename \"%sjpinkexp.png\" -b \"%s\" \"%s\"" % (x0, y0, x1, y1, self.options.density, tmp, bgcol, curfile) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inkscape also has export functions that will use the same Inkscape and environment that is currently in use. I couldn't make it work on Windows, but here's a link that uses it and works on Linux:
https://gitlab.com/Moini/nextgenerator/-/blob/master/next_gen.py#L125
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(If you figure out why this doesn't work on Windows, I'd be very glad for the help, see https://gitlab.com/Moini/nextgenerator/-/issues/5 )
f = p.stdout | ||
err = p.stderr | ||
#inkex.utils.debug("command:" + command) | ||
#inkex.utils.debug("Errorcode:" + str(return_code)) | ||
|
||
def getTmpPath(self): | ||
"""Define the temporary folder path depending on the operating system""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inkex comes with functionality for this.
@@ -19,10 +20,12 @@ This extension requires that imagemagick is installed, more info and download at | |||
<effect needs-live-preview="false"> | |||
<object-type>all</object-type> | |||
<effects-menu> | |||
<submenu _name="Export" /> | |||
<submenu _name="FabLab Chemnitz"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
e = JPEGExport() | ||
e.affect() | ||
e.run() | ||
exit() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The exit() should not be needed.
Btw. would this also be the time to convert the extension to a proper export extension? So it will appear in the 'Save as...' dropdown, instead of in the extensions listing? |
That would be intersting indeed, but for the moment I'd prefer to go ahead with this PR as a MVP to have at least something that works. |
No description provided.