Skip to content

refactored output.py to correct vim behavior#253

Merged
pcottle merged 2 commits into
facebook:masterfrom
brwong:master
Jan 3, 2017
Merged

refactored output.py to correct vim behavior#253
pcottle merged 2 commits into
facebook:masterfrom
brwong:master

Conversation

@brwong
Copy link
Copy Markdown
Contributor

@brwong brwong commented Dec 1, 2016

  • changed vim output from columns to tabs
  • changed vim command so that jump-to-line commands work

(Feel free to modify as needed.)

- changed vim output from columns to tabs
- changed vim command so that jump-to-line commands work
@facebook-github-bot
Copy link
Copy Markdown

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact cla@fb.com if you have any questions.

@facebook-github-bot
Copy link
Copy Markdown

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@brwong
Copy link
Copy Markdown
Contributor Author

brwong commented Dec 1, 2016

My vim command reference, FYI

Stack Overflow - Vim : Open multiples files on different lines

@pcottle
Copy link
Copy Markdown
Contributor

pcottle commented Dec 2, 2016

Hrm, most people at work I talk to prefer columns rather than tabs (just because its immediately clear that $x files are open). And my other worry is that this has been the default behavior for over a year, and changing in this release would be a bit thrashy

would you be open to putting this behind some kind of system flag?

- editor "vim" behavior corrected
  - columns with correct line numbers
- editor "vim -p" behavior changed
  - tabs with correct line numbers
@brwong
Copy link
Copy Markdown
Contributor Author

brwong commented Dec 21, 2016

I changed it so that "vim" acts as before (columns, but with the correct line number jumps), and setting FPP_EDITOR to "vim -p" opens new tabs.

As usual, I'm open to suggestions.

Copy link
Copy Markdown
Contributor

@pcottle pcottle left a comment

Choose a reason for hiding this comment

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

sorry for the delay on this, was offline most of the holidays :) left some comments inline but this looks great, let me pull this down locally and play with it for a bit and then ill probably merge it with some changes

Comment thread scripts/makeDist.sh
DEST="./dist/fpp.$VERSION.tar.gz"
mkdir -p ./dist/
tar -cf $DEST src/*.py fpp
tar -czf $DEST src/*.py fpp
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is czf in trunk and was changed recently. you probably reverted this in some kind of rebase or something -- mind dropping the change to this file?

Comment thread src/output.py
firstFilePath, firstLineNum = filesAndLineNumbers[0]
cmd += ' +%d %s' % (firstLineNum, firstFilePath)
for (filePath, lineNum) in filesAndLineNumbers[1:]:
cmd += ' +"vsp +%d %s"' % (lineNum, filePath)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

woahhh so vsp does vertical split like -O and allows you to specify the line numbers? very cool!

Comment thread src/output.py
return 'vim', 'vim'


def getEditFileCommand(filePath, lineNum):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ah i see why this had to go -- yeah you didnt have global context when composing these mini-parts.

Comment thread src/output.py
cmd += ' +%d %s' % (firstLineNum, firstFilePath)
for (filePath, lineNum) in filesAndLineNumbers[1:]:
cmd += ' +"tabnew +%d %s"' % (lineNum, filePath)
elif editor == 'vim':
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

note to self -- we lost the FPP_DISABLE_SPLIT behavior here so lets add that back

@pcottle pcottle merged commit 062f4ad into facebook:master Jan 3, 2017
pcottle added a commit that referenced this pull request Jan 3, 2017
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.

3 participants