skymakerolof / dxf

DXF parser for node/browser
https://www.npmjs.com/package/dxf
MIT License
335 stars 122 forks source link

Add "ATTRIB" entity support #101

Closed 1egoman closed 2 years ago

1egoman commented 2 years ago

I found that for a DXF parsing task I was doing, I needed access to ATTRIB entities. They seemed to have the same structure as MTEXT entities, so this was mostly just a copy and paste with some slight modifications.

Unfortunately, I don't have a dxf file that can be made public which I could use to add test cases for ATTRIB parsing. If someone would be able to help me out with generating one, or give me some instructions to generate one, I'd be more than willing to add them.

Thanks!

ieskudero commented 2 years ago

This should be an amazing addition to the parser, why is not still merged? If the only requirement is to have a dxf file with an attribute I can maybe create one.

@skymakerolof is this doable?

skymakerolof commented 2 years ago

@ieskudero I think you're right, if you could supply a test file and testcase for this change that would be awesome!

ieskudero commented 2 years ago

@skymakerolof I have created an attribute file, but I think the implementation should be changed. The way I see it there are at least 3 entities that I think they should share a common base text entity (MTEXT, ATTRIB & ATTDEF): MTEXT ATTRIB ATTDEF

If @1egoman agrees, I can start with his implementation and add another pull request with the changes I suggest (create an ATTDEF entity handler and add a base class to those entities). I don't think I will be able to get it all right, since I find some group codes repeated and I don't know why (code 70, for example), but I think I am going to be able to add what is already there.

skymakerolof commented 2 years ago

@ieskudero Even if it isn't perfect, if I understand correctly it should still be better than what we have today. So I'd say go ahead if you feel like you can do it! If there is some weird case, you can add a comment about it in the code.

skymakerolof commented 2 years ago

Closing this, see https://github.com/skymakerolof/dxf/pull/109