provide a fair implementation to compare with in vanilla js

[trvswgnr]

addresses concerns outlined here

[remirth]

+1

[jbanes]

This looks good! I'll review it in detail and get back to you with a code review later today. No major changes that I can see at the moment, and the ones I will request likely can be done without affecting LOC. I'll update the analysis after that.

Key point is that you've updated the coding style and very slightly changed at least one of the semantics. We'll need to update the jQuery version to match, though I can take care of that.

Thanks for sending this over, this is nice work! 😎👍

[datner]

no idea what 'semantics' were changed, regarding the coding style they seemed to have matched your own rather than change them. I used prettier to format mine. Besides a couple of unnecessary indents in the jQuery code theres no difference (no LOC amount change), so the styles match exactly in my PR

[jbanes]

Looks good! Can we change the two items I mentioned (1 bug and 1 state issue) and I'll get this fully integrated. I'm going to go ahead and start putting a structure in place for the before and after so that viewers can see what was done.

[jbanes]

Can you find a better way to carry the page number through the click? Parsing out the display text means that state is now shared between the code and the HTML. Maybe use an attribute like you do on line 41 for the content?

[jbanes]

This text is clickable resulting in a "Page NaN" if clicked. Can you make this non-clickable to avoid this error?

[jbanes]

This is a nice way to provide configurability as well as carry state. It does change the semantics slightly, but this can be adjusted on the jQuery side to match. I'll need to provide some configurability over there so that it's like for like.

[jbanes]

no idea what 'semantics' were changed

I highlighted in the code review

regarding the coding style they seemed to have matched your own rather than change them

Look again. I code Allman style. This is K&R style. Also, he collapsed all the vertical spacing I use around control blocks and related statement types.

[datner]

if anything you almost use One True Brace style in your code, not 'vanilla' K&R. This means that everything is exactly aligned with that style.
If not, then you also don't align with your coding style, as seen
here
here
here
here
here
here
and here

That said, you do not have .editorconfig, .prettierrc, dprint.config.json, biome.json or any other formatter configuration in your project. I don't think theres any substance in pointing out that someone doesn't intuit your specific coding style, especially one thats borrowed from a different language altogether. This is doubly true regarding your preferences for vertical space, be reasonable

[jbanes]

if anything you almost use One True Brace style in your code, not 'vanilla' K&R.

I code Allman, not K&R

This means that everything is exactly aligned with that style. If not, then you also don't align with your coding style, as seen...

All of your examples are consistent with my coding style. Here is a document that explains the style in detail:

https://github.com/jbanes/versus-js/blob/docs/documents/CODESTYLE.md

[datner]

This is even more problematic, as the only definition it provides is about control blocks, which you don't adhere to. Here are some sources for reference: 1 2 3 4 5 6 7 8

These sources might align with your personal coding style, but I need to stress that this style was neither established nor mentioned beforehand. Moreover, it's a style that's hardly ever seen in JavaScript, and you don't strictly adhere to it but rather expand upon it.

It's perfectly okay to have a preferred coding style for your project, but it would be better to admit that you should have clarified the style from the beginning instead of merely stating that it had changed without any explanation. It's time to let go of this issue.

To prevent any dismissive counterarguments, here's an example of the problematic code corrected to the actual Allman style as it is defined and used in practice, without any personal modifications that deviate from the Allman definition:

// curr:
            if(operation === "page" && value) this.data("page", value);
            else if(operation === "page") return this.data("page");
            else if(operation === "pages") return this.data("pages");
// correct Allman style, applicable even for single statements.
// The reason for this is explained in the links I provided:
            if(operation === "page" && value) 
            {
              this.data("page", value);
            }
            else if(operation === "page")
            {
              return this.data("page");
            }
            else if(operation === "pages")
            {
              return this.data("pages");
            }

// curr:
        $.fn.pagedContent = function(operation, value) {
          // ...
        }
// correct Allman style, applicable even for anonymous functions.
// There's a reason why you have to explicitly change this in clang-format:

        $.fn.pagedContent = function(operation, value) 
        {
          // ...
        }

Let's clarify this one more time to ensure understanding. Regardless of the coding style you choose, whether it's Allman or something else, any issues raised about coding style before having a clear style guide can be seen as moving the goalposts and can come across poorly. This is further exacerbated when the style isn't explicitly described, and it's even more problematic when the style is finally named but incorrectly so. However, your incorrect labeling of the style as Allman is a minor detail. It's not the main issue and doesn't really matter. The key concern here is the apparent sudden expectation for everyone to adhere to this style.