Skip to content
This repository has been archived by the owner on Oct 4, 2023. It is now read-only.

Update for InkScape 1.0 #27

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Update for InkScape 1.0 #27

wants to merge 2 commits into from

Conversation

vmario89
Copy link

No description provided.

<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>
Copy link
Contributor

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?

Copy link
Author

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

Copy link
Contributor

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="")
Copy link
Contributor

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="")
Copy link
Contributor

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="")

Copy link
Contributor

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:
Copy link
Contributor

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.

Copy link
Author

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

Copy link
Contributor

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)
Copy link
Contributor

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)
Copy link
Contributor

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?

Copy link
Author

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)
Copy link
Contributor

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

Copy link
Contributor

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"""
Copy link
Contributor

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">
Copy link
Contributor

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()
Copy link
Contributor

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.

@giacmir
Copy link
Owner

giacmir commented Jul 26, 2020

Thanks @vmario89 for your contribution and @Moini for the code review.
Please resolve all the suggested points and then I will create two release branches, one for Inkscape 0.9 until it's still around and one for the new 1.0 version.

@Moini
Copy link
Contributor

Moini commented Jul 26, 2020

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?

@giacmir
Copy link
Owner

giacmir commented Jul 27, 2020

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants