Ah, I see what you are saying. Yes, I see how this wouldn't be a breaking change at all. Sorry for my confusion.
On Tue, Mar 4, 2025, 2:27 AM Mark de Does <m...@mdedoes.xs4all.nl> wrote: > Kevin, Others, > > See my more elaborate response to Andreas Lehmkuehler. > > It is not a breaking change as existing code uses the existing methods, > that are unchanged. > > So let's not rename PDAbstractContentStream. That might be a breaking > change. (I think it is not, but let's stay on the safe side) > > About the name: I noticed that PDContentStream mainly is about reading > content streams. PDAbstractContentStream is about issuing commands to > the stream, so I added *Output* somewhere in the name. For me any name > will do. I agree that the Microsoft convention from the 1990ies to add > an I or include 'Interface' in the name is not a good idea. > > What name do we choose? PDContentInstructionStream? > > Greetings.. Mark > > -- > Mark de Does, Magdalenastraat 6, 3512 NH Utrecht, the Netherlands > <https://www.google.com/maps/search/Magdalenastraat+6,+3512+NH+Utrecht,+the+Netherlands?entry=gmail&source=g> > , > http://maps.google.nl/?q=52.08692,5.1271, > e:m...@mdedoes.xs4all.nl t: 030 2314150 > > On 03-03-2025 15:34, Kevin Day wrote: > > I'm pretty sure introducing an interface is a breaking change at the > > binary level. While it is source compatible, it will require recompile > > for anyone calling the method. > > > > I personally am not a fan of adding "Interface" to the name of > > interfaces (or putting an "I" at the front). > > > > I like PDContentInstructionStream. > > > > If you choose a name like PDContentInstructionStream, you will > > probably also want to change the name of the abstract class to > > PDAbstractContentInstructionStream (or > > AbstractPDContentInstructionStream). The change is already breaking, > > so changing the name of that class is probably ok. > > > > Have you looked carefully at the design from a "single responsibility" > > perspective? I don't know how many methods you pulled up to the > > interface, but if it is a lot of methods, that might indicate a solid > > design review would be a good idea. I'm happy to take a look - but it > > will be a week before I can get to it. > > > > Finally, if backwards compatibility is a high priority, then changing > > the class to not be final would be the best approach. I prefer the > > interface approach, but sometimes backwards compatibility is more > > important than elegant design... > > > > K > > > > > > On Mon, Mar 3, 2025, 4:09 AM Tilman Hausherr <thaush...@t-online.de> > > wrote: > > > > On 28.02.2025 18:07, Mark de Does wrote: > >> > >> Tilman, Others, > >> > >> To avoid reflections about the original intentions, let's go for > >> the interface approach. (Or can /John/ be consulted?) I already > >> typed the code so that does not feel as a waste for me. > >> > >> That resolves some of the issues in my original mail. Lets > >> recuperate: > >> > >> * Do you think introducing an interface is overkill? Simply > >> making PDAbstractContentStream public would achieve the same > >> thing for me. :: We go for the interface approach > >> * I chose the name PDContentStream for the interface. Later on, > >> I discovered it is an unfortunate name. :: I browsed ISO > >> 32000-2:2020. In 7.8.2 (/Content streams/) It defines a > >> content stream as a sequence of instructions. Would > >> PDInstuctionStream be a good name for the interface? > >> * I moved the JavaDoc comments from the abstract class to the > >> interface. You agree? Or shall I also leave them in place? :: > >> Agreed to move? > >> * On which branch must I base my PR? (I see that the trunk > >> already has a 4.0 version number) :: I am jumping to get this > >> now! What can I do to get this in the next 3.something > >> release? (As far as I can judge.. This widens the API > >> interface and does not change anything functionally) > >> > > Hi, > > > > I added a link to the ticket. You're welcome to apply for an > > account - just include something that we know it's you. I agree > > with the javadoc move. PRs should be against the trunk so OK. > > > > I'm not sure if it is a breaking change. I think it isn't, but > > ChatGPT says it is, but reading its reasoning makes me think it > isn't. > > > > I don't like the name, why not PDContentStreamInterface? Yes it is > > confusing that we have a PDContentStream interface. I'd wait a bit > > for other opinions. > > > > John is still member of the project but hasn't been active since > then. > > > > Tilman > > > > > > > >> * > >> > >> > >> > >> > >> Greetings.. Mark. (Sorry for being pushy) > >> > >> -- > >> Mark de Does,Magdalenastraat 6, 3512 NH Utrecht, the Netherlands < > https://www.google.com/maps/search/Magdalenastraat+6,+3512+NH+Utrecht,+the+Netherlands?entry=gmail&source=g > >, > >> http://maps.google.nl/?q=52.08692,5.1271, > >> e:m...@mdedoes.xs4all.nl t: 030 2314150 > >> On 28-02-2025 14:40, Tilman Hausherr wrote: > >>> Making PDAbstractContentStream public seems to be the easier > >>> solution, but: > >>> It was created package-private. I don't know if (1) John had a > >>> specific reason for it or (2) this was just "best practice" to > >>> make stuff restricted until somebody requests it to be > >>> accessible. However he wrote "unless we want users to subclass > >>> PDAbstractContentStream" so he wanted to prevent that, so it > >>> would be (1). That makes me undecided. > >>> > >>> See here: https://issues.apache.org/jira/browse/PDFBOX-4068 > >>> > >>> Amusingly not only this is still open but it was me who created > >>> it 🤦♂️ > >>> > >>> Tilman > >>> > >>> On 28.02.2025 14:22, Mark de Does wrote: > >>>> In one of my projects, I convert (a subset of) SVG to PDF, > >>>> using PdfBox. > >>>> > >>>> In SVG, the contents of the main <svg> element, the <marker>, > >>>> <symbol> and <pattern> definitions have the same syntax and > >>>> structure. With PdfBox 2, I use a PDPageContentStream to record > >>>> the content of all three. With PdfBox 3, that no longer works > >>>> because I must use a PDPatternContentStream to record a > >>>> pattern. Because the different kinds of content stream do not > >>>> share a visible (public) ancestor, I cannot use the same code > >>>> to record SVG content to a content stream. > >>>> > >>>> This morning, I captured the public methods of > >>>> PDAbstractContentStream in an interface and declared > >>>> PDAbstractContentStream to implement that interface. I adapted > >>>> my code to use the interface and everything works fine. (As > >>>> expected). > >>>> > >>>> Before I make a PR, I need some guidance. > >>>> > >>>> * Do you think introducing an interface is overkill? Simply > >>>> making > >>>> PDAbstractContentStream public would achieve the same thing > >>>> for me. > >>>> (The class, not the constructor) > >>>> * I chose the name PDContentStream for the interface. Later on, I > >>>> discovered that is an infortunate name: A different > >>>> interface with > >>>> that name already exists. What would be a better name? > >>>> * I moved the JavaDoc comments from the abstract class to the > >>>> interface. You agree? Or shall I also leave them in place? > >>>> * On which branch must I base my PR? (I see that the trunk > >>>> already has > >>>> a 4.0 version number) > >>>> > >>>> Thanks and Greetings.. Mark > >>>> > >>> > >>> > >>> > --------------------------------------------------------------------- > >>> > >>> To unsubscribe, e-mail: users-unsubscr...@pdfbox.apache.org > >>> For additional commands, e-mail: users-h...@pdfbox.apache.org > >>> > >
