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

Fix adding/replacing a grammar #4097

Merged
merged 4 commits into from
Apr 12, 2018
Merged

Fix adding/replacing a grammar #4097

merged 4 commits into from
Apr 12, 2018

Conversation

lildude
Copy link
Member

@lildude lildude commented Apr 12, 2018

Description

Whilst looking into the problems with #4088, I discovered adding a new grammar broke with the merging of #3982 as follows:

$ script/add-grammar https://github.com/munificent/wren-sublime
Registering new submodule: vendor/grammars/wren-sublime
$ git submodule add -f https://github.com/munificent/wren-sublime vendor/grammars/wren-sublime
$ script/grammar-compiler add vendor/grammars/wren-sublime
Confirming license
$ script/licensed --module vendor/grammars/wren-sublime
  > Caching licenes for linguist:
  >   grammar dependencies:
  > /Users/lildude/github/linguist/vendor/gems/ruby/2.4.0/gems/licensed-1.0.0/lib/licensed/dependency.rb:19:in `initialize': Dependency path vendor/grammars/wren-sublime must be absolute (RuntimeError)
  > 	from script/licensed:26:in `new'
  > 	from script/licensed:26:in `block in dependencies'
  > 	from script/licensed:24:in `map'
  > 	from script/licensed:24:in `dependencies'
  > 	from /Users/lildude/github/linguist/vendor/gems/ruby/2.4.0/gems/licensed-1.0.0/lib/licensed/command/cache.rb:27:in `block (3 levels) in run'
  > 	from /Users/lildude/github/linguist/vendor/gems/ruby/2.4.0/gems/licensed-1.0.0/lib/licensed/command/cache.rb:20:in `map'
  > 	from /Users/lildude/github/linguist/vendor/gems/ruby/2.4.0/gems/licensed-1.0.0/lib/licensed/command/cache.rb:20:in `block (2 levels) in run'
  > 	from /Users/lildude/github/linguist/vendor/gems/ruby/2.4.0/gems/licensed-1.0.0/lib/licensed/command/cache.rb:17:in `chdir'
  > 	from /Users/lildude/github/linguist/vendor/gems/ruby/2.4.0/gems/licensed-1.0.0/lib/licensed/command/cache.rb:17:in `block in run'
  > 	from /Users/lildude/github/linguist/vendor/gems/ruby/2.4.0/gems/licensed-1.0.0/lib/licensed/command/cache.rb:12:in `each'
  > 	from /Users/lildude/github/linguist/vendor/gems/ruby/2.4.0/gems/licensed-1.0.0/lib/licensed/command/cache.rb:12:in `flat_map'
  > 	from /Users/lildude/github/linguist/vendor/gems/ruby/2.4.0/gems/licensed-1.0.0/lib/licensed/command/cache.rb:12:in `run'
  > 	from script/licensed:53:in `<main>'
  > caching vendor/grammars/wren-sublime
$

This was because we missed a spot where we needed to pass in a full repo path to licensed.

Whilst I'm at it, I've also put a quick guard to ensure Docker is installed and running so users get a nicer failure experience:

$ script/add-grammar https://github.com/munificent/wren-sublime
Detecting docker
$ docker ps
  > Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running?
$

I've also updated the CONTRIBUTING.md to point out Docker is required for adding or replacing grammars.

Checklist:

  • I am adding new or changing current functionality
    • I have added or updated the tests for the new or changed functionality.

Copy link
Collaborator

@Alhadis Alhadis left a comment

Choose a reason for hiding this comment

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

Tested locally. It works. 👍

In the sense of not working in a cleaner fashion.

@lildude lildude merged commit cc4da98 into master Apr 12, 2018
@lildude lildude deleted the lildude/fix-adding-grammar branch April 12, 2018 10:14
@Alhadis Alhadis mentioned this pull request Apr 13, 2018
5 tasks
@Alhadis
Copy link
Collaborator

Alhadis commented Apr 13, 2018

@lildude I've removed my irrelevant ramblings and my attempt at "refactoring" I got sidetracked by while learning Ruby and was too sleep-deprived at the time to realise what an absurd mess I was making.

Crap reverted and superseded by a fix that resembles something not written by a rambling lunatic.

Testing locally right now. Pushing in advance out of embarrassment.

f121369120

@github-linguist github-linguist locked as resolved and limited conversation to collaborators Jun 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants