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 working directory property #312

Closed
rejeep opened this Issue Jan 19, 2014 · 14 comments

Comments

Projects
None yet
7 participants
@rejeep
Copy link
Contributor

rejeep commented Jan 19, 2014

As discussed in #308, we should add a property called :cwd that will change the default-directory when running a checker. The property takes a single argument, which is a function that will be called each time the checker runs and it will return a string path.

I guess it makes to fallback and not change the default directory if the function returns nil.

Example:

(flycheck-define-checker foo
  :cwd (lambda ()
         (f-traverse-upwards
          (lambda (path)
            (f-dir? (f-expand ".cask" path)))
          (f-parent (f-this-file)))))

Note that I have noticed that setting the default-directory for a sub-process did not always work unless you specified the absolute path with an ending slash. The function f-full does exactly this.

@lunaryorn

This comment has been minimized.

Copy link
Contributor

lunaryorn commented Jan 20, 2014

I'll work on this.

@ghost ghost assigned lunaryorn Jan 22, 2014

@ananthakumaran

This comment has been minimized.

Copy link

ananthakumaran commented Feb 16, 2014

+1

@rbraunstein

This comment has been minimized.

Copy link

rbraunstein commented Jul 28, 2015

+1

@s3bs

This comment has been minimized.

Copy link
Contributor

s3bs commented Nov 25, 2015

+1, I'm using flycheck with a custom checker. I modified my flycheck sources such that it changes default-directory in the (let ... scope where the checker is executed. For me a static :cwd is sufficient at the moment. Also as discussed in #308 I added functionality to change the process-environment temporary to add environment variables needed by the checker, which are also static.

@lunaryorn

This comment has been minimized.

Copy link
Contributor

lunaryorn commented Nov 25, 2015

@s3bs I'm sorry that you have to go so far as to modify our sources, but currently this feature has a rather low priority on our backlog. To me, #774 and #800 are much more important. They are also much larger and complex than this one, so it'll take a long time before I get to this issue.

I'd love to see a PR for this feature ☺️

@s3bs

This comment has been minimized.

Copy link
Contributor

s3bs commented Nov 25, 2015

@lunaryorn I actually like to tinker once in a while with elisp code. If I'd know more elisp I'd give you a hand. I put my changes that work for me as a fork in my github repository in the cwd branch if that helps anyone in the meantime.

@BernardNotarianni

This comment has been minimized.

Copy link

BernardNotarianni commented Dec 29, 2015

I did modify a checker to change directory using the :predicate property.
Any idea to improve this to make the working directory coming back to the original location?

(with-eval-after-load "flycheck"
  (flycheck-define-checker elixir-mix
    "An Elixir syntax checker using the Elixir interpreter.
     See URL `http://elixir-lang.org/'."
    :command ("mix"
              "compile"
              source)
    :predicate is-mix-project-p
    :error-patterns
    ((error line-start "** (" (zero-or-more not-newline) ") "
            (zero-or-more not-newline) ":" line ": " (message) line-end)
     (warning line-start
              (one-or-more (not (syntax whitespace))) ":"
              line ": "
              (message)
              line-end))
    :modes elixir-mode)
  (add-to-list 'flycheck-checkers 'elixir-mix))

(defun is-mix-project-p ()
  (let ((mix-project-root (locate-dominating-file (buffer-file-name) "mix.exs")))
    (if mix-project-root (cd mix-project-root) nil)))

BernardNotarianni added a commit to BernardNotarianni/ohai-emacs that referenced this issue Dec 30, 2015

Refactor elixir checker to accept phoenix projects
The elixir checker uses the `mix compile` command. This command
has to be executed from the root of the project containing
the `mix.exs` config file.

The commit allow the checker to find this root directory and launch
the command from here, giving access to all libs dependencies.

This is kind of a hack. An issue exist in the flycheck project:
flycheck/flycheck#312
@lunaryorn

This comment has been minimized.

Copy link
Contributor

lunaryorn commented Dec 30, 2015

@BernardNotarianni You may have luck with hooking into flycheck-after-syntax-check-hook, but you're really abusing :predicate here; we'll not support that. As far as I'm concerned I'd strongly advise you to wait for this PR being merged.

If you have further questions about this hack or need further help please let us discuss elsewhere (i.e. in the issue you have referenced). Please let us keep this thread focused on the discussion of this pull request.

Thank you.

@lunaryorn lunaryorn added S-blocked and removed S-ready labels Jan 7, 2016

@lunaryorn lunaryorn closed this Jan 9, 2016

@lunaryorn lunaryorn removed the S-blocked label Jan 9, 2016

@jaseemabid

This comment has been minimized.

Copy link

jaseemabid commented Mar 29, 2016

@lunaryorn Any chance of adding a cwd? rebar3 dialyzer needs to be run at the root of the project as well.

@lunaryorn

This comment has been minimized.

Copy link
Contributor

lunaryorn commented Mar 29, 2016

@jaseemabid We have an open PR at #813 which still needs work and testing afaik.

@jaseemabid

This comment has been minimized.

Copy link

jaseemabid commented Mar 29, 2016

@lunaryorn Ah. Thank you.

@jaseemabid

This comment has been minimized.

Copy link

jaseemabid commented Apr 2, 2016

@lunaryorn In the specific project I'm working on, all source resides in a single src folder and all I have to do is add cd .. && <specific linter command> into ~/bin/foo and set :cmd to foo.

Is there a flycheck way to avoid the external bin file and add cd && into the :cmd somehow?

@lunaryorn

This comment has been minimized.

Copy link
Contributor

lunaryorn commented Apr 2, 2016

@jaseemabid Not currently, no. We do not even have :cwd property currently. I'm sorry.

@lunaryorn

This comment has been minimized.

Copy link
Contributor

lunaryorn commented Sep 23, 2016

Fixed as of #973

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment