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

Enhancements that enable phptidy to be used with external programs #2

Merged
merged 3 commits into from
Jan 31, 2015
Merged

Enhancements that enable phptidy to be used with external programs #2

merged 3 commits into from
Jan 31, 2015

Conversation

sudar
Copy link
Contributor

@sudar sudar commented Jan 27, 2015

Continuing with our discussion from #1 I have made a couple of enhancements that would enable us to use phptidy from external programs

Here is the summary of changes that I did

  • Ability to pass config file as a command line option
  • Added a new option --quiet that will disable printing any messages
  • Added a new command print that can be used to print the modified file instead of saving it.

Let me know if you want me to change anything. Would appreciate a merge. Thanks.

cmrcx added a commit that referenced this pull request Jan 31, 2015
Enhancements that enable phptidy to be used with external programs
@cmrcx cmrcx merged commit 77f2414 into cmrcx:master Jan 31, 2015
@sudar
Copy link
Contributor Author

sudar commented Jan 31, 2015

Thanks for merging it.

Next, I am planning to create a homebrew package for phptidy. I will let you know once I do that.

@cmrcx
Copy link
Owner

cmrcx commented Jan 31, 2015

I'm thinking if it would make sense to separate the output in STDOUT and STDERR. We could send only the formatted code to STDOUT and all messages to STDERR. I think that's the standard way programms should behave.

Then it would be possible to use output redirectors and pipes. For example:
$ phptidy print example.php > example_formatted.php
This would display the messages while writing the output to the file.

What do you think about this?

@cmrcx
Copy link
Owner

cmrcx commented Jan 31, 2015

What's the point in loading an external config file and a directory config file at the same time?

@cmrcx
Copy link
Owner

cmrcx commented Jan 31, 2015

At the moment the print command does exactly the same as the source command. Is it intended, that the print command does not print anything if nothing was changed? In this case the print command is redundant. In the other case, it's a bug.

@sudar
Copy link
Contributor Author

sudar commented Feb 2, 2015

I'm thinking if it would make sense to separate the output in STDOUT and STDERR. We could send only the formatted code to STDOUT and all messages to STDERR. I think that's the standard way programms should behave.

This makes perfect sense. I will implement it.

What's the point in loading an external config file and a directory config file at the same time?

I am calling phptidy using vim autoformat addon external formatter feature. Vim autoformat passes the filename to the external formatter together with external options and expects the formatted file to be printed to stdout.

Since I will be able to pass only the filename, phptidy was not able to read the config file and that's why I added this external config file.

At the moment the print command does exactly the same as the source command. Is it intended, that the print command does not print anything if nothing was changed?

The source command behaves like print command when the file is changed. But the print command still prints even if the file is not changed. The auto-formatter above needs it to behave that way.

I didn't wanted to change the behaviour of existing commands and that's why I implemented print command. If you want I can do remove print command, but add a new flag to source that prints the source code even if it is not changed.

@cmrcx
Copy link
Owner

cmrcx commented Feb 2, 2015

I have already implemented the STDOUT/STDERR thing.

Regarding the config file you did not answer my question. Why do you load the external config file additionally to the directory config file, not instead of it.

@sudar
Copy link
Contributor Author

sudar commented Feb 2, 2015

Regarding the config file you did not answer my question. Why do you load the external config file additionally to the directory config file, not instead of it.

Sorry, I didn't read your question properly first time.

The only reason is that I didn't wanted to change much of existing functionality. Looking back, I think it makes sense to replace the directory config file, if external config file is present. Let me know if you want me to make a pull request for this.

@cmrcx
Copy link
Owner

cmrcx commented Feb 2, 2015

Can you please write a short description, how one could use the vim autoformat addon with phptidy now?

@sudar
Copy link
Contributor Author

sudar commented Feb 3, 2015

Can you please write a short description, how one could use the vim autoformat addon with phptidy now?

Using phptidy with vim autoformat addon

Install vim (or macvim) and then vim autoformat addon

After that add the following lines to ~/.vimrc file

let g:formatprg_php = "/path/to/phptidy.php"
let g:formatprg_args_expr_php = '"print -q -c=/path/to/phptidy-config.php %:p"'

After that open a PHP file in vim and then enter the following command.

:Autoformat

Vim will send the current file name to phptidy and then will replace the current buffer with the formatted content returned by phptidy.

Let me know if you want me to send a pull request with the above description in the readme file.

I am in the process of writing a couple of tutorials about using vim with PHP and I am planning to write a separate article about using phptidy with vim. Will let you know once I do that.

I also just noticed that you have fixed all the issues that were introduced by me. Thanks for doing it and sorry for introducing the issues.

@cmrcx
Copy link
Owner

cmrcx commented Feb 4, 2015

Ok, it works for me.

One issue: If I open a PHP file in vim, change something and then type :Autoformat, then my changes are lost. I have seen in the vim-autoformat description, that a plugin should read from STDIN.

I have noticed, that the path to phptidy.php can be omitted, if it's installed in the PATH. But what to do with the path to the config file? Does one have to edit the .vimrc everytime one goes to a different project?

@sudar
Copy link
Contributor Author

sudar commented Feb 6, 2015

One issue: If I open a PHP file in vim, change something and then type :Autoformat, then my changes are lost. I have seen in the vim-autoformat description, that a plugin should read from STDIN.

Since we are passing the filename from vim to phptidy, we need to save the file before running the :Autoformat command.

have noticed, that the path to phptidy.php can be omitted, if it's installed in the PATH.

Yes. vim just needs the executable. If it is already in PATH, we can ignore it.

But what to do with the path to the config file? Does one have to edit the .vimrc everytime one goes to a different project?

Right now that's a small nuisance that we have. I will let you know if I find a better solution for this.

@cmrcx
Copy link
Owner

cmrcx commented Feb 6, 2015

Is there any reason for using the file name instead of STDIN? I tried to modify phptidy to use STDIN and it seems to work fine.

@sudar
Copy link
Contributor Author

sudar commented Feb 6, 2015

Is there any reason for using the file name instead of STDIN? I tried to modify phptidy to use STDIN and it seems to work fine.

The only reason why I used file name instead of STDIN was just because it was easy. If you have modified phptidy to use STDIN, then we can just use that instead of filename.

One advantage of using STDIN is that we don't have to save the file before running :Autoformat command.

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.

None yet

2 participants