Skip to content

Commit

Permalink
add namespace to unified xml file (fixes KanjiVG#111)
Browse files Browse the repository at this point in the history
  • Loading branch information
@melissaboiko
melissaboiko committed Jun 3, 2015
1 parent 434cc02 commit ea76953
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion kvg.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def release():
out.write(licenseString)
out.write("\nThis file has been generated on %s, using the latest KanjiVG data\nto this date." % (datetime.date.today()))
out.write("\n-->\n")
out.write("<kanjivg>\n")
out.write("<kanjivg xmlns:kvg='http://kanjivg.tagaini.net'>\n")
for f in files:
data = open(os.path.join(datadir, f)).read()
data = data[data.find("<svg "):]
Expand Down

5 comments on commit ea76953

@ospalh
Copy link

@ospalh ospalh commented on ea76953 Jun 4, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a good idea. Thanks.
(Even though this may become obsolete at some time. There is a discussion about how to change the format. Maybe you want to read it, or even participate in it.)

Anyway, some more things:

  • Could you please take care not to mix tabs and spaces in Python files? That files was indented using tabs, now it has one line, your change, that uses spaces with tabs directly before and after it.
  • Could you perhaps turn the change into a pull request?
  • This would require using branches, to separate this change from 434cc02.
  • A pull request for that commit is also welcome.
  • As i think the tabs of the old line 78 should stay (or the whole file detabified (and otherwise cleaned up)) the easiest way to do this may be to roll back this change and put the new change into its own branch.

ETA:

  • Or, as both changes are minor, i could re-do them, (or redo one and cherry-pick the other) if you prefer that.

@Gnurou
Copy link

@Gnurou Gnurou commented on ea76953 Jun 8, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change looks reasonable to me. Leoboiko, could you address the issues raised by ospalh so we can merge it?

@melissaboiko
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, sorry for the trouble.

@ospalh
Copy link

@ospalh ospalh commented on ea76953 Jun 8, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess i will just detabify the file instead and re-do the change. Looking at it now, i want to get rid of the tabs.

@melissaboiko
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh well, I just made the (tabified) pull request. But, as you say, the change is really minor, so feel free to discard it and just do it yourself…

Please sign in to comment.