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

Update travis config, node 10 & dependencies #53

Merged
merged 17 commits into from Apr 12, 2021

Conversation

Nargonath
Copy link
Member

I wasn't sure you'd want to remove the node 8 & 10 versions so I left them here. I believe you'll want to make a breaking change to change that.

@coveralls
Copy link

coveralls commented Sep 25, 2020

Coverage Status

Coverage remained the same at 100.0% when pulling 804ad15 on Nargonath:use-travis-template into dae9481 on hapipal:master.

@devinivy
Copy link
Member

Thanks! This work will be done as a part of #51, so it's fine to make that breaking change here.

@Nargonath
Copy link
Member Author

Alright I'll change that then. I just updated the file since I made a mistake. It seems you can have only one import key in your config file so I updated it to use the array version.

@Nargonath
Copy link
Member Author

There seem to be a timeout on the test for hpal new on Windows with node@14.

@Nargonath Nargonath changed the title Update travis config to use shared config Update travis config, node 10 & dependencies Oct 27, 2020
@Nargonath
Copy link
Member Author

I dug more into updating marked and marked-terminal. marked@1.0.0 introduced breaking changes in the way tokens are structured for composite markdown elements i.e lists, blocks. Previously tokens were a flat array list even for composite elements i.e for a heading with a list and one item you had:

{
  { type: 'heading' },
  { type: 'list_start' },
  { type: 'list_item_start' },
  { type: 'paragraph' },
  { type: 'list_item_end' },
  { type: 'list_end' }
]

However they changed that structure and now composite elements have nested children. Here is how it looks like now for the same structure mentioned above:

{
  { type: 'heading' },
  { type: 'list', tokens: [
    { type: 'list_item', tokens: [
      { type: 'paragraph' } 
    ] } 
  ] }
]

This has impact on the way hpal parses the markdown content hence multiple tests are now failing (~7 tests). I started working on updating the code base to handle the new token format. I started on this function:

else if ((token.type === 'list_item_start') &&
. Just to make sure I understand it correctly, @devinivy can you confirm: we keep the whole content of the first list we encounter which has its first child whose content matches the list item matcher?

For more information, see:

@devinivy
Copy link
Member

@Nargonath Oh my! Haha, I did not anticipate running into changes to marked's lexer, but I suppose one ought to expect the unexpected these days.

Just to make sure I understand it correctly, @devinivy can you confirm: we keep the whole content of the first list we encounter which has its first child whose content matches the list item matcher?

Actually I think the intent is to just pick out the list item and any of its sub-items if it has a deeper list beneath it. Here's an example of what you see from the user's perspective.

Finding a markdown section

❯ npx hpal docs expose         

Searching docs from hapijs/hapi @ master...

### server.expose(key, value, [options])

Used within a plugin to expose a property via server.plugins[name] (#server.plugins) where:

    * key - the key assigned (server.plugins[name][key] (#server.plugins)).
    * value - the value assigned.
    * options - optional settings:
        * scope - controls how to handle the presence of a plugin scope in the name (e.g. @hapi/test):
        ...

Finding a markdown list item
(I believe this is the case you're talking about: you can see it's just the one list item and its sub-items.)

❯ npx hpal docs expose options

Searching docs from hapijs/hapi @ master...

### server.expose(key, value, [options])

    * options - optional settings:
        * scope - controls how to handle the presence of a plugin scope in the name (e.g. @hapi/test):
        ...

@Nargonath
Copy link
Member Author

Alright thanks for the explanation. I should have enough information to proceed with the changes otherwise I'll come back with some more questions. 😄

@devinivy
Copy link
Member

devinivy commented Nov 25, 2020

Hey @Nargonath I ended-up making some headway on the marked updates: 14e51fc

This PR should be ready for merge soon as-is 👍

@Nargonath
Copy link
Member Author

@devinivy Cool! Sorry I didn't get time to go back on it with the GH Actions migration on hapi side. Do you want me to rebase the PR to the v3 branch?

@devinivy
Copy link
Member

@Nargonath actually this is already in the v3 branch! Unfortunately I can't change the base branch of this PR to v3, so let's just let it hang around and close itself when the v3 branch is merged to master. You work here is done, thank you! 💯

@Nargonath
Copy link
Member Author

Oh ok cool. 👍

@devinivy devinivy added this to the 3.0.0 milestone Dec 12, 2020
@devinivy devinivy merged commit 804ad15 into hapipal:master Apr 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants