Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Automated updated of autocomplete-css completions.json #398

Merged
merged 33 commits into from
Mar 15, 2023

Conversation

confused-Techie
Copy link
Member

Alright so this PR turned into a lot more than I had intended it to be.

In short this PR allows autocomplete-css to continue to update it's completions.json as well as getting it updated to today's standards. This was done by completely rewriting the update.coffee and now using update.js

That's the short version, but for some context:


Previously autocomplete-css would use this JSON file located https://raw.githubusercontent.com/adobe/brackets/master/src/extensions/default/CSSCodeHints/CSSProperties.json to update it's completions. This file would provide it with CSS Properties, which it would then manually query MDN for each property, and scrape the webpage for relevant data to build a description.

But as one might imagine this file became out of date, and no longer would provide us with the newest CSS Properties. So rather than try to find someone else that was manually managing this data, I turned to a resource that would keep this data updated for some time, and had at least some reason to keep things backwards compatible when inevitably the data structure changes, hopefully it won't be for some time.

But I ended up using @webref/css NPM GitHub which as they define the standards, they will continue to have the most up to date information we could get. This package provides an easy way to get an abundance of data about every possible specification of CSS. More than we needed, but this would give us what we need.

The two big hurdles:

  1. The data format here was slightly tricky, requiring a bit of code to work around it, but nothing to crazy. The real difficult part was their usage of CSS Value Definition Syntax, which while obvious that they would use it, meant we couldn't easily grab the data needed for our completions.json, that is grab the valid values that each property would accept. Instead of trying to scrape MDN or anything I instead built a super simple parser. This parser is located in cssValueDefinitionSyntaxExtractor.js (A bit obvious and long I know) but this will go ahead and accept any CSS Value Definition Syntax String and parse out all valid values of a property. While this does ignore many aspects of the syntax, such as some values are only valid when another is not present, that was out of the scope of what we needed. And would require significant change on the autocomplete-css package as a whole, even if in the future I'd love to see this being able to utilize the syntax directly and actually provide autocompletions based on that.

  2. The second challenge here was to actually then get descriptions of our properties. This was done utilizing mdn/content which is the GitHub repo that contains the Markdown documents of all of MDN's website. With this we are much more easily able to parse out the needed descriptions from the Markdown rather than having to worry about hitting their website dozens of times and dealing with the overhead of that. A quick note, is these documents are also the same way we are collecting our HTML tags for our completions.json file so those will continue to be updated.

The last note here, our pseudoSelectors are not updated. At this time I was unable to identify the best way to keep these updated, so for the time being we are still using the same ones that we had previously. There is a function that can be used to start updating them, once we determine the best way. But for now that is not being accomplished.

But with all that said, our pseudoSelectors have not been changed, but our properties are now as up to date as the current CSS spec is, and tags are as updated as the MDN website is. So I'd like to think this is a huge improvement over what we previously had, even if I now have ideas for how to improve our package as a whole.


Resolves #393

@DeeDeeG
Copy link
Member

DeeDeeG commented Feb 25, 2023

This is very impressive, I didn't get a chance to review all the JS of the updater script or anything like that, but the spirit of this seems spot on.

Would love to take a closer look if I have time, but it seems pretty in-depth.

Lite approve ✅, I don't mind this merging, just wish I could be more thorough in reviewing it at the moment!

@confused-Techie
Copy link
Member Author

This is very impressive, I didn't get a chance to review all the JS of the updater script or anything like that, but the spirit of this seems spot on.

Would love to take a closer look if I have time, but it seems pretty in-depth.

Lite approve ✅, I don't mind this merging, just wish I could be more thorough in reviewing it at the moment!

Appreciate it! Feel free to ask any questions about it that you are curious about. This one did end up being way more complicated than originally intended

@confused-Techie
Copy link
Member Author

So with all the changes in, and all of the first issues people noticed having been resolved. I've moved on to looking at the tests: Which are not what we are hoping for:

  • autocomplete-css: 125 Failures - We expect 0
  • find-and-replace: 66 Failures - Tad High but within expected range
  • symbols-view: 2 Failures - Expected
  • tree-view: 2 Failures - Expected

So obviously we have many failures within the tests of autocomplete-plus that we do not expect. To be frank I haven't run tests, as the structure of the completions.json hasn't changed, so there should be zero change in how the tests run. That is unless of course there's any manual checking for values suggested or returned that no longer match up with the updated data. So I'll go ahead and continue to take a look at this.

@DeeDeeG
Copy link
Member

DeeDeeG commented Feb 25, 2023

Some of it, IIRC from trying similar a while ago, was a hard-coded expected number of properties or values or some such assertion. That'll need updating.

Perhaps various other checks assume old values/properties.

@confused-Techie
Copy link
Member Author

Some of it, IIRC from trying similar a while ago, was a hard-coded expected number of properties or values or some such assertion. That'll need updating.

Perhaps various other checks assume old values/properties.

Yeah, it's a little strange. Essentially the vast majority that are failing are expecting the top level completions that would be returned. For example border- would expect the first result to be border: and the second result to be border-radius

Which honestly checks out. This makes sense that you would expect this rather than, what we are currently getting, background-blend-mode and background-position. Thing is, obviously I didn't change any handling of this, which to me means one of two things. The completions.json is expected to be ordered in some such way, since the values returned are truly just the top items that begin with a b, or that the end all be all of our completions relies on the function firstCharsEqual() within provider.coffee. Which that does exactly what it sounds like. Compares if the first characters match between a completion candidate and the buffer text. Either way though that may mean I need to apply some fixes to autocomplete-css.

But honestly as I don't know coffeescript, or the existing code here, I'm hoping the change could be simple. Possible as simple as ordering the completions before being passed back to the user manually, such as I suggested on the linked issue in my PR description using LCS to rank the results then return them. Although I'd honestly much rather see a change like that land in autocomplete-plus instead of here. But I'll keep trying to determine what exactly has gone wrong

@DeeDeeG
Copy link
Member

DeeDeeG commented Feb 25, 2023

Not sure exactly what the "ordered completions" file was about, that they went through the trouble of referring to usage statistics and place more popularly used properties/values first, based on crawling the actual websites of the web and inspecting the properties/values used most.

This PR deletes those files, so I was going to assume they had only been used for generating the list of completions, and yet the ordering bit feels like it would be unnecessary in that case... So I'm not sure where, but I get a feeling the "ordered completions" file(s) may still be needed... ?? Or some ordering they provided as a middle step before ordering the final output is important?

@confused-Techie
Copy link
Member Author

@DeeDeeG Just wow. Yeah looking back through the source, and sorted-property-names.json I think this is exactly what is happening.

The order of sorted-property-names.json

  1. Matches the order for the completions order that's expected within the tests
  2. Is the canonical order that the completions.json would be written in.

A side note that is just unbelievable, seemingly if a property was retrieved via the API that didn't exist within sorted-property-names.json it would be discarded. Meaning that Atom had always had a static list of accepted properties. So the old update method makes very little sense at this point.

But what that does mean, is the semi-random order of sorted-property-names.json is important, and is causing the specs to fail. But like you said, how in the world did they order it? I refuse to believe it was done based on usage or popularity, since that is pure insanity. I'm hoping rather it's built according to the way autocomplete-plus calculates autocompletions. Such as to be the most streamlined method. But I'm not to sure.


An interesting note. I tried running the new completions.json file locally, and I couldn't replicate the order being retrieved via the specs manually at all.

So I thought, maybe autocomplete-plus does have some code that attempts to order the suggested completions, so activating that package as well in our specs does get 10 or so more tests passing now. From 125 to 116 iirc.

But yeah otherwise that's just a mess of behavior.

I'm wondering how poorly performance would be if we stop just using firstCharsEqual() since to me it seems that's whats trigger the extra completions. But I honestly might want to implement some LCS functionality here, and try to test if performance is impacted. But that was something I'd at least hoped to talk in another PR. Since otherwise I feel I'd have to match the order, or change the specs to expect what's now being returned.

@DeeDeeG
Copy link
Member

DeeDeeG commented Feb 25, 2023

sorted-property-names.json is updated manually - take a look at https://developer.microsoft.com/en-us/microsoft-edge/platform/usage/ and https://www.chromestatus.com/metrics/css/popularity for guidance.

From the autocomplete-css README.md.

@confused-Techie
Copy link
Member Author

sorted-property-names.json is updated manually - take a look at https://developer.microsoft.com/en-us/microsoft-edge/platform/usage/ and https://www.chromestatus.com/metrics/css/popularity for guidance.

From the autocomplete-css README.md.

Wow, when you look everywhere but the readme. Thanks for the link, but that's just gross. Gotta find a solution to that one way or another

`update.js` now will not overwrite any duplicated property entries. In the case of duplication found will write only the one with the most values.

Additionally values are now dedupe prior to adding.

Finally we now support Non-Terminal Data Types in CSS Value Definition Syntax, allowing us to resolve values when the value group refers to another property
@confused-Techie
Copy link
Member Author

Alright, all tests are now finally passing locally on my machine.

Essentially the tests for autocomplete-plus were mostly a validation of the completions.json file and it's order and it's properties. Where nearly every single test would test the amount of completions returned for any property, which of course now is larger. Where they would test the exact order of completions provided, which of course has no changed. Or some even tested the text of the description provided by a completion, by of course checking it's exact index within the completions array provided.

So with that said there's a few things I've done. Unfortunately this makes the tests less readable when they fail, but does mean they will fail significantly less often.

In the case of checking the length of completions that would be provided for a character, since of course now there are more completions every single check failed expecting a smaller number than we now have. While there was a whole discussion about the best choice here over on Discord in the end I've opted to do the simplest thing that can hopefully protect against regression. Checked if the amount of completions we have is higher than what was previously expected. This at least ensures the test will pass, that is until they start removing items from the CSS spec faster than they add them. Which I don't personally think is something we need to be super worried about.

The other options discussed where ensuring the amount is over 0, or seeing if it's within a certain range. But in the end I do personally feel this is the best solution.

But now for the case where the tests would check the exact order of completions.

By this I mean there were endless statements like expect(completions[0].text).toBe 'display: ' Which the thing is, the reason this mostly worked was because of sorted-property-names.json. Apparently previously the Atom team had used a manually made list of completions, that was taken from Microsoft Edge web usage statistics and would rank the properties in order of popularity. And that is how they would then test, to ensure items were still ranked in terms of popularity. Thing is, not sure if this wasn't a feature at the time, but from my understanding and experience is autocomplete-plus actually helps to order the completions according to what most closely matches what a user typed, and beyond that I don't think we actually care about the list returned. If we do, or if it's a problem that when a user types just d and they don't get back the most commonly used items of the language we could revisit this, but I personally think that entire system is prone to failure, and way more trouble than it's worth.

So in that case I've rewritten the majority of tests (whatever was failing) to stop doing this. From what I rewrote we no longer expect any exact order. All we do is check if the value we want to test for exists somewhere in the completions. This is done with isValueInCompletions() at the top of the spec file. And using it we can just check if display: exists in one of the completions provided, rather than checking is it the first completion provided. Granted if values are removed or the spec changes drastically this could mean aspects here have to be rewritten, but for the most part I think this is a safe compromise of ensuring we have the values we expect, but allowing the completions.json file to change over time.

I will admit I hadn't wanted to touch specs on this PR, and assumed I wouldn't have to. But from here the last thing to do is remove the note about sorted-property-list.json within the readme and wait for all tests to pass, which like I said it does on my machine.


Side Note: One last thing I had to add was inserting the implicitly defined CSS Values onto all properties, this did cause our completions.json file to become much larger, but got many values that we tests for back into the file. Such as inherit

@DeeDeeG
Copy link
Member

DeeDeeG commented Mar 5, 2023

Note: We also have some draft properties like ruby-merge from somewhere.

https://drafts.csswg.org/css-ruby-1/#collapsed-ruby

This didn't have a description in the completions.json either, beyon the empty string "".

(There sure are a lot of CSS properties out there!)

@confused-Techie
Copy link
Member Author

Note: We also have some draft properties like ruby-merge from somewhere.

https://drafts.csswg.org/css-ruby-1/#collapsed-ruby

This didn't have a description in the completions.json either, beyon the empty string "".

Yeah looking at all the empty descriptions, and have gotten nearly all of them solved locally, just taking a look at a last few ones, but should be up within 30 minutes hopefully

packages/autocomplete-css/completions.json Outdated Show resolved Hide resolved
packages/autocomplete-css/completions.json Outdated Show resolved Hide resolved
@confused-Techie
Copy link
Member Author

Alright, just pushed some changes, now I'll have to correct myself, when I said "nearly all of them" turns out that was a lie lol.
Got the majority in a consecutive block, but truth be told there are still 225 properties without any descriptions.

Doing a spot check on those remaining I am able to find them listed in their respective official spec although not on MDN.

Really all this whole ordeal is teaching me is maybe I should start contributing to MDN for some of these undocumented elements.

If you'd also like to do a spot check on any remaining values the best thing to do is:

  1. Find the respective property within the source file of all properties.
  2. Use the spec.url at the top of the file that contains your mystery property
  3. Browse the specification there to find the relevant properties.

@DeeDeeG
Copy link
Member

DeeDeeG commented Mar 5, 2023

Some of the -webkit prefixed ones are weirdly forever-prefixed per a "compat" specification: https://compat.spec.whatwg.org/#css-simple-aliases

I am looking through the update.js code and the data structures from W3C / MDN stuff and seeing if I can't figure out how to at least grab which properly standard (non-prefixed) property name they are an alias of.

Mostly it's the prefixed name minus the -webkit- part, but a few like -webkit-mask-box-image-outset are "find -webkit-mask-box replace with mask-border".

Maybe we should programatically generate description strings that just say "alias of: [non-prefixed property name here]"?

Except for -webkit-background-size which is technically slightly different from background-size, according to: whatwg/compat#28

@confused-Techie
Copy link
Member Author

Some of the -webkit prefixed ones are weirdly forever-prefixed per a "compat" specification: https://compat.spec.whatwg.org/#css-simple-aliases

I am looking through the update.js code and the data structures from W3C / MDN stuff and seeing if I can't figure out how to at least grab which properly standard (non-prefixed) property name they are an alias of.

Mostly it's the prefixed name minus the -webkit- part, but a few like -webkit-mask-box-image-outset are "find -webkit-mask-box replace with mask-border".

Maybe we should programatically generate description strings that just say "alias of: [non-prefixed property name here]"?

Except for -webkit-background-size which is technically slightly different from background-size, according to: whatwg/compat#28

Yeah I've been looking around for a solid solution here.

The only thing I can see to find is either we can build a manually maintained list based off the resource you provided, or we accept all of those attributes having no description.

Since there are many vendor prefixed items that do have proper descriptions, but many that don't.

At this point it really does make it tempting to try and build a much more intelligent parser for this sort of thing, as it's hard to believe no single resource has the information we need in a machine parsable way.

But which do you think is better then? Leaving as is or a manually updated list, since we could do either. And if a manual list can always default to assuming it's its own valid vendor prefixed item

@DeeDeeG
Copy link
Member

DeeDeeG commented Mar 7, 2023

I got this list of remaining properties with empty description "":

Properties with empty "description" (click to expand):
% git branch --show-current
update-autocomplete-css
% git rev-parse HEAD
732a353f45f9216133ddc5695361defc25e46e2e
> for (const prop in completions.properties) { if (completions.properties[prop].description === "") { console.log(prop) };  };

-webkit-align-items
-webkit-align-content
-webkit-align-self
-webkit-animation-name
-webkit-animation-duration
-webkit-animation-timing-function
-webkit-animation-iteration-count
-webkit-animation-direction
-webkit-animation-play-state
-webkit-animation-delay
-webkit-animation-fill-mode
-webkit-animation
-webkit-backface-visibility
-webkit-background-clip
-webkit-background-origin
-webkit-background-size
-webkit-border-bottom-left-radius
-webkit-border-bottom-right-radius
-webkit-border-top-left-radius
-webkit-border-top-right-radius
-webkit-border-radius
-webkit-box-shadow
-webkit-box-sizing
-webkit-flex
-webkit-flex-basis
-webkit-flex-direction
-webkit-flex-flow
-webkit-flex-grow
-webkit-flex-shrink
-webkit-flex-wrap
-webkit-filter
-webkit-justify-content
-webkit-mask
-webkit-mask-box-image-outset
-webkit-mask-box-image-repeat
-webkit-mask-box-image-slice
-webkit-mask-box-image-source
-webkit-mask-box-image-width
-webkit-mask-clip
-webkit-mask-image
-webkit-mask-origin
-webkit-mask-position
-webkit-mask-repeat
-webkit-mask-size
-webkit-order
-webkit-perspective
-webkit-perspective-origin
-webkit-transform-origin
-webkit-transform-style
-webkit-transform
-webkit-transition-delay
-webkit-transition-duration
-webkit-transition-property
-webkit-transition-timing-function
-webkit-transition
-webkit-text-size-adjust
-webkit-box-align
-webkit-box-flex
-webkit-box-ordinal-group
-webkit-box-orient
-webkit-box-pack
grid-row-gap
grid-column-gap
grid-gap
anchor-scroll
anchor-name
anchor-default
position-fallback
background-position-inline
background-position-block
corner-shape
corners
border-limit
border-clip
border-clip-top
border-clip-right
border-clip-bottom
border-clip-left
margin-break
color-adjust
string-set
bookmark-level
bookmark-label
bookmark-state
reading-order
layout-order
wrap-flow
wrap-through
font-synthesis-weight
font-synthesis-style
font-synthesis-small-caps
copy-into
footnote-display
footnote-policy
object-view-box
baseline-source
text-edge
leading-trim
inline-sizing
initial-letter-wrap
line-grid
line-snap
box-snap
link-parameters
marker-side
spatial-navigation-contain
spatial-navigation-action
spatial-navigation-function
overflow-clip-margin-top
overflow-clip-margin-right
overflow-clip-margin-bottom
overflow-clip-margin-left
overflow-clip-margin-block-start
overflow-clip-margin-inline-start
overflow-clip-margin-block-end
overflow-clip-margin-inline-end
overflow-clip-margin-inline
overflow-clip-margin-block
block-ellipsis
line-clamp
continue
max-lines
float-reference
float-defer
float-offset
page
flow-into
flow-from
region-fragment
block-step-size
block-step-insert
block-step-align
block-step-round
block-step
border-boundary
ruby-merge
ruby-overhang
scroll-start
scroll-start-target
scroll-start-x
scroll-start-y
scroll-start-inline
scroll-start-block
shape-inside
shape-padding
min-intrinsic-sizing
voice-volume
voice-balance
speak
speak-as
pause-before
pause-after
pause
rest-before
rest-after
rest
cue-before
cue-after
cue
voice-family
voice-rate
voice-pitch
voice-range
voice-stress
voice-duration
word-boundary-detection
word-boundary-expansion
text-space-collapse
text-space-trim
hyphenate-limit-zone
hyphenate-limit-lines
hyphenate-limit-last
word-wrap
text-wrap
wrap-before
wrap-after
wrap-inside
text-align-all
text-group-align
line-padding
text-spacing
text-decoration-trim
text-decoration-skip-self
text-decoration-skip-box
text-decoration-skip-spaces
text-emphasis-skip
caret-shape
caret
nav-up
nav-right
nav-down
nav-left
input-security
-webkit-user-select
-webkit-appearance
view-transition-name
fill-break
fill-color
fill-image
fill-origin
fill-position
fill-size
fill-repeat
stroke-align
stroke-break
stroke-dash-corner
stroke-dash-justify
stroke-color
stroke-image
stroke-origin
stroke-position
stroke-size
stroke-repeat
view-timeline-name
view-timeline-axis
view-timeline-inset
view-timeline
animation-range
animation-range-start
animation-range-end
stroke-alignment
stroke-dashcorner
stroke-dashadjust
shape-subtract
marker


The majority are not prefixed.


I think there's been a lot of progress made with this PR, though:

% wc -l before_completions.txt after_completions.txt empty_description.txt 
     237 before_completions.txt
     673 after_completions.txt
     225 empty_description.txt

With this PR, we have more new properties with descriptions (673 - 225 = 448) than there used to be completions, period. autocomplete-css specs are passing in CI again, now that d property has a non-empty description.

I'm thinking this is good enough, and if we want to get descriptions for the remaining properties that can be follow-up work? But I will say, with you having taken the initiative to do this, I would want to defer to your opinion about how hard to try and get descriptions for all (or at least most) of the remaining properties. It seems kinda tricky to me, TBH, but I think we could figure out the rest of them eventually at the current pace (especially since a few are from the same spec). It'd just take some time. Maybe a fair amount of time.

@DeeDeeG
Copy link
Member

DeeDeeG commented Mar 7, 2023

At this point it really does make it tempting to try and build a much more intelligent parser for this sort of thing, as it's hard to believe no single resource has the information we need in a machine parsable way.

I dunno, there's this I guess? https://www.w3.org/TR/CSS/#properties

Not super machine-readable, but maybe scrape-able and we can go from there? Maybe the source of this page is checked in to a repo somewhere? Hmm. I see it's checked in here: https://github.com/w3c/csswg-drafts/blob/main/css-2023/Overview.bs#L1045

But there is some magic to it, because it doesn't have anywhere near all the content in that file. I assume a lot is added during the build. I think their CI uses some custom W3C tool called bikeshed. https://github.com/w3c/csswg-drafts/blob/main/.github/workflows/build-specs.yml#L54 And they publish the generated artifact(s) of that straight to GitHub Pages' servers is seems like, not saved to a branch, so the final page and source I linked to seem to be it for the formats this resource is published as.

@confused-Techie
Copy link
Member Author

@DeeDeeG yeah so looking through the properties left I was able to automatically collect one more. But beyond that it seems none of these items exist on MDN to the best of my ability, so limit our ability to collect them automatically. Despite my best efforts in doing so.

So for the time being for some of these last elements I may just go ahead and build a small manual list of descriptions to include.

Like we have determined having empty descriptions is not the end of the world, so while in the future this list will not be all inclusive, we can still automatically update these without concern. Adding in the newly empty descriptions is just a cherry on top that wouldn't at all be required. So I'll try to submit a PR with those changes soon, and add what I can. But after all of this I may decide to take the time to try and find a way to get specs like this automatically updated elsewhere so that we here can rely on it, and hopefully anywhere else as well. Since it does seem crazy to me that this isn't easily possible as of right now.

@confused-Techie
Copy link
Member Author

@DeeDeeG alright, so I've tried my best to curb the issue of many empty property descriptions.

Like mentioned above the best way I could see us doing this, is unfortunately a manual list. So I've gone ahead and implemented this behavior, where as a last resort our manual list is checked for a property description.

Once the update.js runs it will actually warn you how many empty descriptions you have. With an optional flag of --show-empty being available to print every single property with a missing description.


I've now added many manual descriptions either copy/pasted from the source spec or manually written using the source spec as a guide to do so. This has now gotten our empty completion descriptions down from 225 to 90. So big wins there, but I think I've read through as many CSS specs as I can for the day. So either, I'd be more than happy to go ahead and merge this as is, and at a later date anyone can pick this up to finish the descriptions that need to be made manually, or if we really do want them included I can try to get to this again soon, or really anyone else can as well, comitting directly to my branch. But let me know what you think.

The remaining empty description completions

margin-break
color-adjust
string-set
bookmark-level
bookmark-label
bookmark-state
reading-order
layout-order
wrap-flow
wrap-through
font-synthesis-weight
font-synthesis-style
font-synthesis-small-caps
copy-into
footnote-display
footnote-policy
object-view-box
baseline-source
text-edge
leading-trim
inline-sizing
initial-letter-wrap
line-grid
line-snap
box-snap
link-parameters
marker-side
spatial-navigation-contain
spatial-navigation-action
spatial-navigation-function
overflow-clip-margin-top
overflow-clip-margin-right
overflow-clip-margin-bottom
overflow-clip-margin-left
overflow-clip-margin-block-start
overflow-clip-margin-inline-start
overflow-clip-margin-block-end
overflow-clip-margin-inline-end
overflow-clip-margin-inline
overflow-clip-margin-block
block-ellipsis
line-clamp
continue
max-lines
float-reference
float-defer
float-offset
page
flow-into
flow-from
region-fragment
block-step-size
block-step-insert
block-step-align
block-step-round
block-step
border-boundary
ruby-merge
ruby-overhang
scroll-start
scroll-start-target
scroll-start-x
scroll-start-y
scroll-start-inline
scroll-start-block
shape-inside
shape-padding
min-intrinsic-sizing
text-space-collapse
text-decoration-trim
text-decoration-skip-self
text-decoration-skip-box
text-decoration-skip-spaces
text-emphasis-skip
caret-shape
caret
nav-up
nav-right
nav-down
nav-left
input-security
view-transition-name
fill-color
view-timeline-name
view-timeline-axis
view-timeline-inset
view-timeline
animation-range
animation-range-start
animation-range-end

@confused-Techie
Copy link
Member Author

@DeeDeeG just an fyi, I've now updated manual-property-desc.json to include every single missing description.

So there should be zero empty descriptions now.

Copy link
Member

@DeeDeeG DeeDeeG left a comment

Choose a reason for hiding this comment

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

Okay, thank you for this huge effort. Now with all the descriptions filled out, there's nothing left to object to here, IMO.

I tested the latest binary from this PR, and it's working for me still 👍.

Tried doing node update.js and got equal lines added/removed, looks like the order just shuffled around (I assume this might be an OS thing? I'm on Linux right now). So, completions appear to be stable (other than some descriptions were slightly updated, maybe from the mdn repo -- EDIT: I am not really concerned about getting these in to merge this PR, it would be okay to, it would be okay not to. There will always be MDN contributions, we can't realistically stay 100% up to date on these.).

I see there are indeed no properties without descriptions. Thank you again for this huge effort.

I have one comment below about a cosmetic/docs type suggestion of which stuff to log when running node update.js. I don't care strongly about that, it is just something I noticed, feel free to apply it or ignore it/decline to apply it.

Approved for the above reasons, and as mentioned in earlier approvals in the thread.

packages/autocomplete-css/update.js Outdated Show resolved Hide resolved
Co-authored-by: DeeDeeG <DeeDeeG@users.noreply.github.com>
@confused-Techie
Copy link
Member Author

Thanks a ton to everyone for all the help on this one, and to @DeeDeeG for being so persistent with making sure this got reviewed and had an eye kept on it. Appreciate all of you, and with the above approval I'll go ahead and merge!

@confused-Techie confused-Techie merged commit d8b0a33 into master Mar 15, 2023
@confused-Techie confused-Techie deleted the update-autocomplete-css branch April 22, 2023 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace autocomplete-css and autocomplete-html update paths
4 participants