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

Document how to integrate in pre-commit with plugins #7407

Closed
yajo opened this issue Jan 21, 2020 · 10 comments
Closed

Document how to integrate in pre-commit with plugins #7407

yajo opened this issue Jan 21, 2020 · 10 comments
Labels
area:cli Issues with Prettier's Command Line Interface locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. scope:external This is not an issue with Prettier, it’s an issue with external software, like an editor integration

Comments

@yajo
Copy link

yajo commented Jan 21, 2020

Environments:

  • Prettier Version: 1.19.1
  • Running Prettier via: pre-commit
  • Runtime: node v12.13.1
  • Operating System: Linux

Steps to reproduce:

All I want to do is to integrate the prettier-xml plugin into pre-commit.

I tried with this, but it doesn't work:

  - repo: https://github.com/prettier/prettier
    rev: "1.19.1"
    hooks:
      - id: prettier
        name: prettier xml plugin
        additional_dependencies:
          - "@prettier/plugin-xml@0.5.0"
        args:
          - --with-node-modules
        files: \.xml$

Expected behavior: Reformat xml files

Actual behavior:

Running pre-commit run -a:

prettier xml plugin......................................................Failed
hookid: prettier

[error] No parser could be inferred for file: web_responsive/views/web.xml
[error] No parser could be inferred for file: web_responsive/static/src/xml/apps.xml
[error] No parser could be inferred for file: web_notify/views/res_users_demo.xml
[error] No parser could be inferred for file: web_responsive/static/src/xml/form_view.xml
[error] No parser could be inferred for file: web_notify/views/web_notify.xml
[error] No parser could be inferred for file: web_tree_many2one_clickable/views/assets.xml
[error] No parser could be inferred for file: web_dialog_size/static/src/xml/web_dialog_size.xml
[error] No parser could be inferred for file: web_environment_ribbon/view/base_view.xml
[error] No parser could be inferred for file: web_dialog_size/templates/assets.xml
[error] No parser could be inferred for file: web_ir_actions_act_view_reload/views/web_ir_actions_act_view_reload.xml
[error] No parser could be inferred for file: web_responsive/views/assets.xml
[error] No parser could be inferred for file: web_responsive/views/res_users.xml
[error] No parser could be inferred for file: web_responsive/static/src/xml/navbar.xml
[error] No parser could be inferred for file: web_environment_ribbon/data/ribbon_data.xml
[error] No parser could be inferred for file: web_decimal_numpad_dot/views/web_decimal_numpad_dot.xml
[error] No parser could be inferred for file: web_widget_bokeh_chart/views/web_widget_bokeh_chart.xml

Workaround:

Configuration from prettier/plugin-xml#17 (comment) works, but I guess that's not what you want to recommend to use.

@thorn0
Copy link
Member

thorn0 commented Jan 21, 2020

Where does pre-commit install additional_dependencies to? You need to pass that location via --plugin-search-dir. (Also, I don't think you need --with-node-modules. You don't format files located inside node_modules, do you?)

@thorn0
Copy link
Member

thorn0 commented Jan 21, 2020

Here's another idea: try passing --plugin @prettier/plugin-xml.

@thorn0 thorn0 added the status:awaiting response Issues that require answers to questions from maintainers before action can be taken label Jan 21, 2020
@yajo
Copy link
Author

yajo commented Jan 23, 2020

Where does pre-commit install additional_dependencies to? You need to pass that location via --plugin-search-dir.

pre-commit handles environments automatically in its cache dir, and they're named differently each time. However, if I add additional_dependencies, those should be available to the running environment.

Here's another idea: try passing --plugin @prettier/plugin-xml.

Tried, but no luck:

- repo: https://github.com/prettier/prettier
  rev: "1.19.1"
  hooks:
    - id: prettier
      name: prettier with xml plugin
      args:
        - --plugin=@prettier/plugin-xml
      additional_dependencies:
         - "@prettier/plugin-xml@0.5.0"
      files: \.xml$

fails with:

prettier with xml plugin.................................................Failed
hookid: prettier

/home/yajo/.cache/pre-commit/repo6hmixzon/node_modules/resolve/lib/sync.js:76
    throw err;
    ^

Error: Cannot find module '@prettier/plugin-xml' from '/home/yajo/Documentos/prodevel/doodba-devel-13.0/odoo/custom/src/web'
    at Function.module.exports [as sync] (/home/yajo/.cache/pre-commit/repo6hmixzon/node_modules/resolve/lib/sync.js:74:15)
    at /home/yajo/.cache/pre-commit/repo6hmixzon/src/common/load-plugins.js:40:29
    at Array.map (<anonymous>)
    at loadPlugins (/home/yajo/.cache/pre-commit/repo6hmixzon/src/common/load-plugins.js:33:61)
    at Object.getSupportInfo (/home/yajo/.cache/pre-commit/repo6hmixzon/src/index.js:21:16)
    at updateContextOptions (/home/yajo/.cache/pre-commit/repo6hmixzon/src/cli/util.js:937:35)
    at pushContextPlugins (/home/yajo/.cache/pre-commit/repo6hmixzon/src/cli/util.js:970:3)
    at updateContextArgv (/home/yajo/.cache/pre-commit/repo6hmixzon/src/cli/util.js:981:3)
    at Object.createContext (/home/yajo/.cache/pre-commit/repo6hmixzon/src/cli/util.js:922:3)
    at Object.run (/home/yajo/.cache/pre-commit/repo6hmixzon/src/cli/index.js:8:24) {
  code: 'MODULE_NOT_FOUND'
}
/home/yajo/.cache/pre-commit/repo6hmixzon/node_modules/resolve/lib/sync.js:76
    throw err;
    ^

Error: Cannot find module '@prettier/plugin-xml' from '/home/yajo/Documentos/prodevel/doodba-devel-13.0/odoo/custom/src/web'
    at Function.module.exports [as sync] (/home/yajo/.cache/pre-commit/repo6hmixzon/node_modules/resolve/lib/sync.js:74:15)
    at /home/yajo/.cache/pre-commit/repo6hmixzon/src/common/load-plugins.js:40:29
    at Array.map (<anonymous>)
    at loadPlugins (/home/yajo/.cache/pre-commit/repo6hmixzon/src/common/load-plugins.js:33:61)
    at Object.getSupportInfo (/home/yajo/.cache/pre-commit/repo6hmixzon/src/index.js:21:16)
    at updateContextOptions (/home/yajo/.cache/pre-commit/repo6hmixzon/src/cli/util.js:937:35)
    at pushContextPlugins (/home/yajo/.cache/pre-commit/repo6hmixzon/src/cli/util.js:970:3)
    at updateContextArgv (/home/yajo/.cache/pre-commit/repo6hmixzon/src/cli/util.js:981:3)
    at Object.createContext (/home/yajo/.cache/pre-commit/repo6hmixzon/src/cli/util.js:922:3)
    at Object.run (/home/yajo/.cache/pre-commit/repo6hmixzon/src/cli/index.js:8:24) {
  code: 'MODULE_NOT_FOUND'
}
/home/yajo/.cache/pre-commit/repo6hmixzon/node_modules/resolve/lib/sync.js:76
    throw err;
    ^

Error: Cannot find module '@prettier/plugin-xml' from '/home/yajo/Documentos/prodevel/doodba-devel-13.0/odoo/custom/src/web'
    at Function.module.exports [as sync] (/home/yajo/.cache/pre-commit/repo6hmixzon/node_modules/resolve/lib/sync.js:74:15)
    at /home/yajo/.cache/pre-commit/repo6hmixzon/src/common/load-plugins.js:40:29
    at Array.map (<anonymous>)
    at loadPlugins (/home/yajo/.cache/pre-commit/repo6hmixzon/src/common/load-plugins.js:33:61)
    at Object.getSupportInfo (/home/yajo/.cache/pre-commit/repo6hmixzon/src/index.js:21:16)
    at updateContextOptions (/home/yajo/.cache/pre-commit/repo6hmixzon/src/cli/util.js:937:35)
    at pushContextPlugins (/home/yajo/.cache/pre-commit/repo6hmixzon/src/cli/util.js:970:3)
    at updateContextArgv (/home/yajo/.cache/pre-commit/repo6hmixzon/src/cli/util.js:981:3)
    at Object.createContext (/home/yajo/.cache/pre-commit/repo6hmixzon/src/cli/util.js:922:3)
    at Object.run (/home/yajo/.cache/pre-commit/repo6hmixzon/src/cli/index.js:8:24) {
  code: 'MODULE_NOT_FOUND'
}
/home/yajo/.cache/pre-commit/repo6hmixzon/node_modules/resolve/lib/sync.js:76
    throw err;
    ^

Error: Cannot find module '@prettier/plugin-xml' from '/home/yajo/Documentos/prodevel/doodba-devel-13.0/odoo/custom/src/web'
    at Function.module.exports [as sync] (/home/yajo/.cache/pre-commit/repo6hmixzon/node_modules/resolve/lib/sync.js:74:15)
    at /home/yajo/.cache/pre-commit/repo6hmixzon/src/common/load-plugins.js:40:29
    at Array.map (<anonymous>)
    at loadPlugins (/home/yajo/.cache/pre-commit/repo6hmixzon/src/common/load-plugins.js:33:61)
    at Object.getSupportInfo (/home/yajo/.cache/pre-commit/repo6hmixzon/src/index.js:21:16)
    at updateContextOptions (/home/yajo/.cache/pre-commit/repo6hmixzon/src/cli/util.js:937:35)
    at pushContextPlugins (/home/yajo/.cache/pre-commit/repo6hmixzon/src/cli/util.js:970:3)
    at updateContextArgv (/home/yajo/.cache/pre-commit/repo6hmixzon/src/cli/util.js:981:3)
    at Object.createContext (/home/yajo/.cache/pre-commit/repo6hmixzon/src/cli/util.js:922:3)
    at Object.run (/home/yajo/.cache/pre-commit/repo6hmixzon/src/cli/index.js:8:24) {
  code: 'MODULE_NOT_FOUND'
}

@no-response no-response bot removed the status:awaiting response Issues that require answers to questions from maintainers before action can be taken label Jan 23, 2020
@alexander-akait
Copy link
Member

alexander-akait commented Jan 23, 2020

I think you need to open an issue in the pre-commit repo, it is little bit out of scope prettier

@thorn0 thorn0 added the area:cli Issues with Prettier's Command Line Interface label Jan 23, 2020
@thorn0
Copy link
Member

thorn0 commented Jan 23, 2020

pre-commit handles environments automatically in its cache dir, and they're named differently each time.

Turns out the path to the directory we need can be found in the NODE_VIRTUAL_ENV environment variable (or in NODE_PATH). If only we could do something like --plugin-search-dir=$NODE_VIRTUAL_ENV/lib in the args, that would solve the issue. But looks like pre-commit doesn't support variable expansion.

@thorn0 thorn0 added the scope:external This is not an issue with Prettier, it’s an issue with external software, like an editor integration label Jan 23, 2020
@thorn0 thorn0 mentioned this issue Feb 4, 2020
4 tasks
@yajo
Copy link
Author

yajo commented Feb 6, 2020

ping @asottile @chriskuehl @struys any of you could help resolving our doubts here please? Thanks

@asottile
Copy link
Contributor

asottile commented Feb 6, 2020

Here's a reproduction which does not involve pre-commit. I've packaged it up as a Dockerfile so it's easy to reproduce -- I believe prettier should be resolving peer dependencies using the filesystem (similar to how eslint resolves plugins)?

FROM ubuntu:bionic
RUN : \
    && apt-get update \
    && DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \
        ca-certificates \
        curl \
        dumb-init \
        gnupg2 \
        software-properties-common \
    && apt-get clean \
    && rm -rf /var/lib/apt/lists/*
RUN : \
    && curl -sSL https://deb.nodesource.com/gpgkey/nodesource.gpg.key | apt-key add - \
    && echo "deb https://deb.nodesource.com/node_12.x bionic main" > /etc/apt/sources.list.d/nodesource.list \
    && apt-get update \
    && DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \
        nodejs \
    && apt-get clean \
    && rm -rf /var/lib/apt/lists/*
RUN npm install -g prettier@1.19.1 @prettier/plugin-xml@0.5.0
RUN echo '<x/>' > t.xml
CMD ["dumb-init", "prettier", "--plugin=@prettier/plugin-xml", "t.xml"]

build:

docker build -t test .

run:

docker run --rm -ti test

output:

$ docker run --rm -ti test
/usr/lib/node_modules/prettier/bin-prettier.js:15954
  throw err;
  ^

Error: Cannot find module '@prettier/plugin-xml' from '/'
    at Function.sync (/usr/lib/node_modules/prettier/bin-prettier.js:15952:13)
    at /usr/lib/node_modules/prettier/bin-prettier.js:44147:29
    at Array.map (<anonymous>)
    at loadPlugins (/usr/lib/node_modules/prettier/bin-prettier.js:44139:59)
    at Object.getSupportInfo (/usr/lib/node_modules/prettier/bin-prettier.js:44211:16)
    at updateContextOptions (/usr/lib/node_modules/prettier/bin-prettier.js:46186:33)
    at pushContextPlugins (/usr/lib/node_modules/prettier/bin-prettier.js:46213:3)
    at updateContextArgv (/usr/lib/node_modules/prettier/bin-prettier.js:46224:3)
    at Object.createContext (/usr/lib/node_modules/prettier/bin-prettier.js:46176:3)
    at Object.run (/usr/lib/node_modules/prettier/bin-prettier.js:46257:24) {
  code: 'MODULE_NOT_FOUND'
}

@yajo
Copy link
Author

yajo commented Feb 14, 2020

IMHO Prettier, as a code formatter that supports pre-commit and plugins, should handle this case and make sure both can be used together. 🤔

@thorn0
Copy link
Member

thorn0 commented Mar 23, 2020

fixed by #7508

This works for me:

- repo: https://github.com/prettier/prettier
  rev: "2.0.2"
  hooks:
    - id: prettier
      additional_dependencies:
        - "@prettier/plugin-xml@0.7.2"
      args:
        - --plugin=@prettier/plugin-xml
      files: \.xml$

@thorn0 thorn0 closed this as completed Mar 23, 2020
@asottile
Copy link
Contributor

awesome! thanks a ton @fisker and @thorn0 🎉

@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Jun 24, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Jun 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area:cli Issues with Prettier's Command Line Interface locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. scope:external This is not an issue with Prettier, it’s an issue with external software, like an editor integration
Projects
None yet
Development

No branches or pull requests

4 participants