Skip to content
This repository has been archived by the owner on Apr 1, 2020. It is now read-only.

<f12> opens in current tab, but it seems like it should not #2272

Closed
marene opened this issue May 31, 2018 · 8 comments
Closed

<f12> opens in current tab, but it seems like it should not #2272

marene opened this issue May 31, 2018 · 8 comments

Comments

@marene
Copy link
Contributor

marene commented May 31, 2018

Oni Version: 0.3.4
Neovim Version (Linux only): 0.2.3-843-gf5b0f5e
Operating System: Ubuntu 16.04.3 LTS

Describe your issue

When hitting f12 while the cursor is on a symbol, it goes to that symbol's definition, but in the same tab as the one currently being opened.

Expected behaviour

This was bugging me (because i felt like it should have been going to those definitions in a new tab, much more practical imo), so i had a little peek in the code, and found this: https://github.com/onivim/oni/blob/master/browser/src/Editor/NeovimEditor/Definition.ts#L53
Based on the Enum definition here: https://github.com/onivim/oni/blob/master/browser/src/Editor/NeovimEditor/Definition.ts#L13, it looks like f12 should be opening in a new tab, but is not.

Steps to reproduce

Get your cursor over a symbol usage, then hit f12

Proposed solution

I feel like solving this might be quite easy: just change this line (https://github.com/onivim/oni/blob/master/browser/src/Editor/NeovimEditor/Definition.ts#L59-L60) to:

default:
  return 'tabe'
@oni-bot
Copy link

oni-bot bot commented May 31, 2018

Hello and welcome to the Oni repository! Thanks for opening your first issue here. To help us out, please make sure to include as much detail as possible - including screenshots and logs, if possible.

@CrossR
Copy link
Member

CrossR commented May 31, 2018

Makes sense to me! I've hit this before as well.

I feel like solving this might be quite easy: just change this line (https://github.com/onivim/oni/blob/master/browser/src/Editor/NeovimEditor/Definition.ts#L59-L60) to:

I think that would fix it for people who are using vim style tabs, but would potentally be awkward for people who use buffers option for their tabs. (That is every open buffer has its own tab).

We'd instead probably want to use this function :

public async openFile(
file: string,
openOptions: Oni.FileOpenOptions = Oni.DefaultFileOpenOptions,
): Promise<Oni.Buffer> {
const tabsMode = this._configuration.getValue("tabs.mode") === "tabs"
const cmd = new Proxy(
{
[Oni.FileOpenMode.NewTab]: "tabnew!",
[Oni.FileOpenMode.HorizontalSplit]: "sp!",
[Oni.FileOpenMode.VerticalSplit]: "vsp!",
[Oni.FileOpenMode.Edit]: tabsMode ? "tab drop" : "e!",
[Oni.FileOpenMode.ExistingTab]: "e!",
},
{
get: (target: { [cmd: string]: string }, name: string) =>
name in target ? target[name] : "e!",
},
)
await this._neovimInstance.command(
`:${cmd[openOptions.openMode]} ${this._escapeSpaces(file)}`,
)
return this.activeBuffer
}

Since it deals with that for us and does tabs for most users, and buffers for those who don't.

@marene
Copy link
Contributor Author

marene commented May 31, 2018

Good point.
I did try to test it out, but had some troubles making my build work, I'll give it a fair shot when I'm not at work

@TalAmuyal
Copy link
Collaborator

TalAmuyal commented Jun 1, 2018 via email

@marene
Copy link
Contributor Author

marene commented Jun 4, 2018

If i ever get time to change that, I'll make sure to make it togglable, yes :)

@badosu
Copy link
Collaborator

badosu commented Aug 20, 2018

@psxpaul Does #2504 fix this? If so, how would one configure it?

@psxpaul
Copy link
Contributor

psxpaul commented Aug 20, 2018

Yes, @badosu this should be fixed with my change. The default language.gotoDefinition command should use Oni.DefaultFileOpenOptions to determine where to open. You can change this by adding to your config.tsx:

Oni.DefaultFileOpenOptions.openMode = Oni.FileOpenMode.NewTab

The valid values are Edit, NewTab, VerticalSplit, HorizontalSplit, and ExistingTab`. My change also adds the following commands, which should behave as you expect (equivalent horizontal and vertical commands already exist):

  • language.gotoDefinition.openNewTab
  • language.gotoDefinition.openEdit
  • language.gotoDefinition.openExistingTab

@akinsho
Copy link
Member

akinsho commented Aug 23, 2018

Going to close this now since @psxpaul's PR was merged please feel free to reopen it if it doesn't quite solve the problem

@akinsho akinsho closed this as completed Aug 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants