Hey, I finally got round to creating a PR (creating a draft until I can add more notes)
notes.md
These are just things of note I found while looking through the code
sudoku.js & sudoku2.html
A copy of the original so I can comment on it in the PR (I'm assuming this relatively close to GitHub's PR system)
sudoku.modified.js & sudoku2.modified.html
These contain some improvements
sudoku.rewrite.js & sudoku2.rewrite.html
These are a rewrite I was tempted to make as a fun challenge, hope you like 'em
sudoku.rewrite.css
I wanted to have the css out of the file for editor convenience, of course, it could just as well be in-lined like before to avoid the flash of content
Hey, I finally got round to creating a PR (creating a draft until I can add more notes)
- notes.md
- These are just things of note I found while looking through the code
- sudoku.js & sudoku2.html
- A copy of the original so I can comment on it in the PR (I'm assuming this relatively close to GitHub's PR system)
- sudoku.modified.js & sudoku2.modified.html
- These contain some improvements
- sudoku.rewrite.js & sudoku2.rewrite.html
- These are a rewrite I was tempted to make as a fun challenge, hope you like 'em
- sudoku.rewrite.css
- I wanted to have the css out of the file for editor convenience, of course, it could just as well be in-lined like before to avoid the flash of content
A pretty standard, pretty robust way of handling events. I'd lean to the built-in EventTarget until the need for something better is needed.
A pretty standard, pretty robust way of handling events. I'd lean to the built-in [EventTarget](https://developer.mozilla.org/en-US/docs/Web/API/EventTarget) until the need for something better is needed.
Unfortunately, this doesn't work with dynamic inside out sizing/nested aspect-ration on firefox. Luckily, you can use em since the size is based on font size
Unfortunately, this doesn't work with dynamic inside out sizing/nested aspect-ration on firefox. Luckily, you can use `em` since the size is based on font size
Never! Google can claw my faux-privacy from my cold dead hands (also ublock). I've been considering using chromium/vavaldi so I can get functional HDR (right now I use edge when I need chrome)
Never! Google can claw my faux-privacy from my cold dead hands (also ublock). I've been considering using chromium/vavaldi so I can get functional HDR (right now I use edge when I need chrome)
You can combine styles as well, I find it to be a bit easier to work with on average since you can toggle and combine them when it's needed much easier.
Also CSS now supports nested styling rules:
You can combine styles as well, I find it to be a bit easier to work with on average since you can toggle and combine them when it's needed much easier.
Also CSS now supports nested styling rules:
```css
.sudoku-field {
&.initial {
color: #777;
}
&.selected {
background-color: lightgreen;
}
&.marked {
background-color: blue;
}
&.invalid {
color: red;
}
}
```
Not sure if the double border was intentional, I assume it wasn't.
The way I solved it was making the border on the bottom and right, you can use the nth selector to disable it on the edges if need be
Not sure if the double border was intentional, I assume it wasn't.
The way I solved it was making the border on the bottom and right, you can use the nth selector to disable it on the edges if need be
I'm not sure about that. Will look it up, but you kinda want to have the component ready right when it get 'connected'? I also use that event somewhere I guess.
I'm not sure about that. Will look it up, but you kinda want to have the component ready right when it get 'connected'? I also use that event somewhere I guess.
In the class constructor, you can set up initial state and default values, register event listeners and perhaps create a shadow root. At this point, you should not inspect the element's attributes or children, or add new attributes or children. See Requirements for custom element constructors and reactions for the complete set of requirements.
That's what the connected event is for. They break that "rule" in the example on the same page, and yeah, it gets called through the _bind function. I think the comment was originally meant to be on the _bind call in the constructor
It's here: https://developer.mozilla.org/en-US/docs/Web/API/Web_components/Using_custom_elements
> In the class constructor, you can set up initial state and default values, register event listeners and perhaps create a shadow root. At this point, you should not inspect the element's attributes or children, or add new attributes or children. See Requirements for custom element constructors and reactions for the complete set of requirements.
That's what the connected event is for. They break that "rule" in the example on the same page, and yeah, it gets called through the `_bind` function. I think the comment was originally meant to be on the `_bind` call in the constructor
Oh fuck, that can maybe be the reason something is not working or why even the second variable is implemented. Because I really don't know what the reason was.
Oh fuck, that can maybe be the reason something is not working or why even the second variable is implemented. Because I really don't know what the reason was.
@BordedDev nice review so far! I will check your new files tomorrow. I'm now fighting with speech recognition and command execution. It's a cool project and it was open source for a while but then i really, really start to rape the code. But check how easy it is to add functions to the LLM:
The docstrings are instructions for the LLM. I have somewhere a class inspector converting that nice to LLM commands.
So, if i need new funcitonality, I just add it there.
class ChatFunctions:
def __init__(self, gpt):
self._gpt = gpt
"""Functions callable by GPT"""
async def uptime(self,*args):
"""
Execute this function if it has to do with uptime of server or if use wants to know how long a server is up.
:return: str
"""
result = subprocess.check_output(["ssh","retoor@molodetz.nl","uptime","-p","&&", "printf 'Since:'","&&","uptime","-s"]).decode()
result = await self._gpt.communicate(f"Explain the uptime of this server using this data: {result}")
await self._gpt.tts3(result)
async def deploy(self,*args):
"""
Execute this function if it has to do with deploy, deployment, update, upgrade
:return: str
"""
await self._gpt.tts3("Deploying molodetz.nl")
response = subprocess.check_output("ssh molodetz.nl deploy".split(" ")).decode()
print(response)
await self._gpt.tts3(response)
return False
@BordedDev nice review so far! I will check your new files tomorrow. I'm now fighting with speech recognition and command execution. It's a cool project and it was open source for a while but then i really, really start to rape the code. But check how easy it is to add functions to the LLM:
The docstrings are instructions for the LLM. I have somewhere a class inspector converting that nice to LLM commands.
So, if i need new funcitonality, I just add it there.
```
class ChatFunctions:
def __init__(self, gpt):
self._gpt = gpt
"""Functions callable by GPT"""
async def uptime(self,*args):
"""
Execute this function if it has to do with uptime of server or if use wants to know how long a server is up.
:return: str
"""
result = subprocess.check_output(["ssh","retoor@molodetz.nl","uptime","-p","&&", "printf 'Since:'","&&","uptime","-s"]).decode()
result = await self._gpt.communicate(f"Explain the uptime of this server using this data: {result}")
await self._gpt.tts3(result)
async def deploy(self,*args):
"""
Execute this function if it has to do with deploy, deployment, update, upgrade
:return: str
"""
await self._gpt.tts3("Deploying molodetz.nl")
response = subprocess.check_output("ssh molodetz.nl deploy".split(" ")).decode()
print(response)
await self._gpt.tts3(response)
return False
```
Regex strikes again 🏃💨. Technically inefficient, practically I like it.
BTW it's intentionally not an array it returns so that you can efficiently stream the results through the iterator (that's how I understand the below)
>With matchAll() available, you can avoid the while loop and exec with g. Instead, you get an iterator to use with the more convenient for...of, array spreading, or Array.from() constructs:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/matchAll
I'd normally use a @property for this but it wasn't working the initial value so I did this instead
https://developer.mozilla.org/en-US/docs/Web/CSS/@property
```css
@property --border-radius {
syntax: "<length>";
inherits: true;
initial-value: 5px;
}
```
This is mostly personal preference, since an "industry standard" way would be key => action enum => (action transform =>) action function. I find this a bit easier to read. If I'd bring in a library for the matching I'd have used cond from lodash https://lodash.com/docs/4.17.15#cond
This is mostly personal preference, since an "industry standard" way would be key => action enum => (action transform =>) action function. I find this a bit easier to read. If I'd bring in a library for the matching I'd have used `cond` from lodash https://lodash.com/docs/4.17.15#cond
Type hints are here since I can see them in my IDE and make things slightly easier to work with, hence why I've put them everywhere but with no actual docs
Type hints are here since I can see them in my IDE and make things slightly easier to work with, hence why I've put them everywhere but with no actual docs
This mainly to enforce that the underlying grid isn't modifiable, technically we return a new array anyway, so there is little point. #activeState is also always replaced so we can freeze it when it's created, but this felt more flexible
This mainly to enforce that the underlying grid isn't modifiable, technically we return a new array anyway, so there is little point. `#activeState` is also always replaced so we can freeze it when it's created, but this felt more flexible
I'm being a little cheeky here, making the multiplication an addition instead. I can see an argument that this should be a 1 to 3 loop with multiplication, I liked the simplicity of positionIndex this way
I'm being a little cheeky here, making the multiplication an addition instead. I can see an argument that this should be a 1 to 3 loop with multiplication, I liked the simplicity of `positionIndex` this way
These return a Set to simplify/combine the logic for automatically resolving and checking if a number is valid. In theory, the check would be faster since it can check early
These return a `Set` to simplify/combine the logic for automatically resolving and checking if a number is valid. In theory, the check would be faster since it can check early
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Hey, I finally got round to creating a PR (creating a draft until I can add more notes)
@@ -0,0 +18,4 @@return false}class EventHandler {A pretty standard, pretty robust way of handling events. I'd lean to the built-in EventTarget until the need for something better is needed.
@@ -0,0 +237,4 @@}}class Row extends EventHandler {The Row/Col classes are confusing in the current situation, since Row isn't used for more than a bag holder/event forwarding
@@ -0,0 +228,4 @@}}toString() {Seeing someone implement a custom
toStringis always nice@@ -0,0 +287,4 @@states = []parsing = false_initialized = falseinitalized = falseWhy are there 2 initialized properties?
Even worse, one is wrongly written :P
@@ -0,0 +678,4 @@-ms-user-select: none;background-color: #e5e5e5;border-radius: 5px;aspect-ratio: 1/1;Unfortunately, this doesn't work with dynamic inside out sizing/nested aspect-ration on firefox. Luckily, you can use
emsince the size is based on font sizeSo they're not nice squares on firefox? Upgrade to chrome!
Never! Google can claw my faux-privacy from my cold dead hands (also ublock). I've been considering using chromium/vavaldi so I can get functional HDR (right now I use edge when I need chrome)
@@ -0,0 +680,4 @@border-radius: 5px;aspect-ratio: 1/1;}.sudoku-field-initial {You can combine styles as well, I find it to be a bit easier to work with on average since you can toggle and combine them when it's needed much easier.
Also CSS now supports nested styling rules:
Ah facks. Thanks. Yh, my css skills are very outdated.
Still better than my junior. I was very surprised, learning about the CSS upgrade
thislast year@@ -0,0 +693,4 @@color: red;}.sudoku-field {border: 1px solid #ccc;Not sure if the double border was intentional, I assume it wasn't.
The way I solved it was making the border on the bottom and right, you can use the nth selector to disable it on the edges if need be
@@ -0,0 +767,4 @@this.fieldElements = []this.puzzle = new Puzzle(9)this.fields = []this.styleElement = document.createElement('style');From what I've read, it's not recommended to create elements in the constructor and instead should be done in the
connectedCallbackI'm not sure about that. Will look it up, but you kinda want to have the component ready right when it get 'connected'? I also use that event somewhere I guess.
It's here: https://developer.mozilla.org/en-US/docs/Web/API/Web_components/Using_custom_elements
That's what the connected event is for. They break that "rule" in the example on the same page, and yeah, it gets called through the
_bindfunction. I think the comment was originally meant to be on the_bindcall in the constructor@@ -0,0 +13,4 @@}on(event, listener) {this.events[event] ||= { name: name, listeners: [], callCount: 0 }Simplified the empty event handler assignment. It was meant to be
??=not||=both will work in this case because of you how JS works though@@ -0,0 +43,4 @@if (this.suppresEvents) return [];this.eventCount++;const returnValue = this.events[event].listeners.map(listener => {var returnValue = listener(data)var :(
Oh no! A VAR!
@@ -0,0 +348,4 @@reset() {this._initialized = falsethis.initalized == false;Accidental comparison 😅
Oh fuck, that can maybe be the reason something is not working or why even the second variable is implemented. Because I really don't know what the reason was.
@BordedDev nice review so far! I will check your new files tomorrow. I'm now fighting with speech recognition and command execution. It's a cool project and it was open source for a while but then i really, really start to rape the code. But check how easy it is to add functions to the LLM:
The docstrings are instructions for the LLM. I have somewhere a class inspector converting that nice to LLM commands.
So, if i need new funcitonality, I just add it there.
@@ -0,0 +6,4 @@There is a bunch of unused codeAutosolver is broken, I had to move the function aroundAh, sad. I didn't know status of the code. But if it's working again, i will put the sudoku on my site.
@@ -0,0 +30,4 @@** @type {(number|null|undefined)[]}*/#activeState = new Array(9 * 9)Ooooh, magic numbers :P Did i have that too?
Dang it I missed it, it's meant to be
SUDOKU_GRID_SIZE. You did with thenew Puzzle(9)Nice review so far!
@@ -0,0 +91,4 @@||field.row.index == this.row.index && field.value == this._value)}).filter(field => field != this).length == 0A double filter, it's inefficient (since both will loop through the array) but for the size it doesn't really matter
@@ -0,0 +295,4 @@this.debugEvents = true;this.initalized = falsethis.rows = []if (isNaN(arg)) {Nice usage of the coercion, even if my IDE complains because there is coercion
@@ -0,0 +391,4 @@this._initialized = false;const regex = /\d/g;const matches = [...content.matchAll(regex)]Regex strikes again 🏃💨. Technically inefficient, practically I like it.
BTW it's intentionally not an array it returns so that you can efficiently stream the results through the iterator (that's how I understand the below)
Hey @BordedDev come talk at https://rock.molodetz.nl.
@@ -0,0 +1,73 @@:host {display: inline-block;--border-radius: 5px;I'd normally use a @property for this but it wasn't working the initial value so I did this instead
https://developer.mozilla.org/en-US/docs/Web/CSS/@property
@@ -0,0 +3,4 @@max = Math.floor(max)return Math.floor(Math.random() * (max - min + 1)) + min}I ran Biome.js over this, and I'm a
;hater (my default config), that's why they're not here@@ -0,0 +248,4 @@this.puzzle = puzzlethis.cols = []this.index = this.puzzle.rows.lengthconst me = thisA lot of these
meproxies aren't necessary with the arrow functions you're using, they're removed in the modified version@@ -0,0 +300,4 @@return invalid[invalid.length - 1]}for (const row of this.rows) {Switched
.forEachtofor ofloop for performance (recommended practice, but honestly, it’s not a big deal - I'd even call it a nitpick)@@ -0,0 +365,4 @@const matches = [...content.matchAll(regex)]const max = this.size * this.sizefor (const [index, match] of matches.entries()) {Collapsed the index to come from the
matchesarray, since I noted the 1-to-1 relation@@ -0,0 +769,4 @@} else {const keyLookup = {u() {puzzle.popState()This is mostly personal preference, since an "industry standard" way would be key => action enum => (action transform =>) action function. I find this a bit easier to read. If I'd bring in a library for the matching I'd have used
condfrom lodash https://lodash.com/docs/4.17.15#cond@@ -0,0 +19,4 @@.sudoku-field {aspect-ratio: 1 / 1;width: 1em;line-height: 1;This is to fix the vertical alignment since it had a value of 1.5 for me
@@ -0,0 +13,4 @@* @param min {number}* @param max {number}* @returns {number}*/Type hints are here since I can see them in my IDE and make things slightly easier to work with, hence why I've put them everywhere but with no actual docs
@@ -0,0 +29,4 @@** @type {SudokuState[]}*/#state = []This contains a stack of states, essentially the initial state + all steps
@@ -0,0 +43,4 @@*/get grid() {const gridValue = [...this.#activeState]Object.freeze(gridValue)This mainly to enforce that the underlying grid isn't modifiable, technically we return a new array anyway, so there is little point.
#activeStateis also always replaced so we can freeze it when it's created, but this felt more flexible@@ -0,0 +75,4 @@for (const char of serializedState) {if (PARSE_RANGE_START <= char && char <= PARSE_RANGE_END) {if (VALID_RANGE_START <= char && char <= PARSE_RANGE_END) {Since 0 is an invalid value for the
SudokuStatethis allows for them to be filtered out while still increasing the number placement@@ -0,0 +95,4 @@* @returns {undefined|number}*/#sampleRandomEmptyField(stateToSample = this.#activeState) {const iters = [...stateToSample.entries().filter(([, v]) => v == null)]Recent learned that the variables are optional in destruction, had them as
_before.@@ -0,0 +126,4 @@const left = Math.trunc(Math.trunc(slot % 9) / 3) * 3const top = Math.trunc(Math.trunc(slot / 9) / 3) * (9 * 3)for (let y = 0; y <= 27; y += 9) {I'm being a little cheeky here, making the multiplication an addition instead. I can see an argument that this should be a 1 to 3 loop with multiplication, I liked the simplicity of
positionIndexthis way@@ -0,0 +184,4 @@* @param state {SudokuState}* @return {Set<number>}*/getValues(slot, state = this.#activeState) {These return a
Setto simplify/combine the logic for automatically resolving and checking if a number is valid. In theory, the check would be faster since it can check early@@ -0,0 +268,4 @@}class SudokuHost extends HTMLElement {static observedAttributes = ["size", "puzzle", "readonly"]These were necessary for me to have
attributeChangedCallbackwork@@ -0,0 +304,4 @@#keyProcessors = new Map()get isActive() {return this.matches(":hover")This was more convenient for me than the mouseenter/mouseexit events
@@ -0,0 +425,4 @@this.#styling = document.createElement("link")this.#styling.rel = "stylesheet"this.#styling.href = "sudoku.rewrite.css"this.#root.appendChild(this.#styling)This was more convenient for me than replacing the outer HTML with a
<style>...</style>string to get syntax highlighting30062089f6to77e78c8031