Commit d9ec3bfc authored by Phil Hughes's avatar Phil Hughes

Jump to definition in the same file

Closes https://gitlab.com/gitlab-org/gitlab/-/issues/213189
parent fbce9ed5
...@@ -8,7 +8,12 @@ export default { ...@@ -8,7 +8,12 @@ export default {
Popover, Popover,
}, },
computed: { computed: {
...mapState(['currentDefinition', 'currentDefinitionPosition', 'definitionPathPrefix']), ...mapState([
'currentDefinition',
'currentDefinitionPosition',
'currentBlobPath',
'definitionPathPrefix',
]),
}, },
mounted() { mounted() {
this.body = document.body; this.body = document.body;
...@@ -44,5 +49,6 @@ export default { ...@@ -44,5 +49,6 @@ export default {
:position="currentDefinitionPosition" :position="currentDefinitionPosition"
:data="currentDefinition" :data="currentDefinition"
:definition-path-prefix="definitionPathPrefix" :definition-path-prefix="definitionPathPrefix"
:blob-path="currentBlobPath"
/> />
</template> </template>
<script> <script>
import { GlDeprecatedButton } from '@gitlab/ui'; import { GlButton } from '@gitlab/ui';
export default { export default {
components: { components: {
GlDeprecatedButton, GlButton,
}, },
props: { props: {
position: { position: {
...@@ -18,6 +18,10 @@ export default { ...@@ -18,6 +18,10 @@ export default {
type: String, type: String,
required: true, required: true,
}, },
blobPath: {
type: String,
required: true,
},
}, },
data() { data() {
return { return {
...@@ -32,9 +36,18 @@ export default { ...@@ -32,9 +36,18 @@ export default {
}; };
}, },
definitionPath() { definitionPath() {
return ( if (!this.data.definition_path) {
this.data.definition_path && `${this.definitionPathPrefix}/${this.data.definition_path}` return null;
); }
if (this.isDefinitionCurrentBlob) {
return `#${this.data.definition_path.split('#').pop()}`;
}
return `${this.definitionPathPrefix}/${this.data.definition_path}`;
},
isDefinitionCurrentBlob() {
return this.data.definition_path.indexOf(this.blobPath) === 0;
}, },
}, },
watch: { watch: {
...@@ -77,9 +90,15 @@ export default { ...@@ -77,9 +90,15 @@ export default {
</p> </p>
</div> </div>
<div v-if="definitionPath" class="popover-body"> <div v-if="definitionPath" class="popover-body">
<gl-deprecated-button :href="definitionPath" target="_blank" class="w-100" variant="default"> <gl-button
:href="definitionPath"
:target="isDefinitionCurrentBlob ? null : '_blank'"
class="w-100"
variant="default"
data-testid="go-to-definition-btn"
>
{{ __('Go to definition') }} {{ __('Go to definition') }}
</gl-deprecated-button> </gl-button>
</div> </div>
</div> </div>
</template> </template>
...@@ -52,7 +52,8 @@ export default { ...@@ -52,7 +52,8 @@ export default {
return; return;
} }
const data = state.data[blobEl.dataset.path]; const blobPath = blobEl.dataset.path;
const data = state.data[blobPath];
if (!data) return; if (!data) return;
...@@ -72,6 +73,6 @@ export default { ...@@ -72,6 +73,6 @@ export default {
setCurrentHoverElement(el); setCurrentHoverElement(el);
} }
commit(types.SET_CURRENT_DEFINITION, { definition, position }); commit(types.SET_CURRENT_DEFINITION, { definition, position, blobPath });
}, },
}; };
...@@ -15,8 +15,9 @@ export default { ...@@ -15,8 +15,9 @@ export default {
[types.REQUEST_DATA_ERROR](state) { [types.REQUEST_DATA_ERROR](state) {
state.loading = false; state.loading = false;
}, },
[types.SET_CURRENT_DEFINITION](state, { definition, position }) { [types.SET_CURRENT_DEFINITION](state, { definition, position, blobPath }) {
state.currentDefinition = definition; state.currentDefinition = definition;
state.currentDefinitionPosition = position; state.currentDefinitionPosition = position;
state.currentBlobPath = blobPath;
}, },
}; };
...@@ -4,4 +4,5 @@ export default () => ({ ...@@ -4,4 +4,5 @@ export default () => ({
data: null, data: null,
currentDefinition: null, currentDefinition: null,
currentDefinitionPosition: null, currentDefinitionPosition: null,
currentBlobPath: null,
}); });
...@@ -32,9 +32,10 @@ document.addEventListener('DOMContentLoaded', () => { ...@@ -32,9 +32,10 @@ document.addEventListener('DOMContentLoaded', () => {
GpgBadges.fetch(); GpgBadges.fetch();
if (gon.features?.codeNavigation) { const codeNavEl = document.getElementById('js-code-navigation');
const el = document.getElementById('js-code-navigation');
const { codeNavigationPath, blobPath, definitionPathPrefix } = el.dataset; if (gon.features?.codeNavigation && codeNavEl) {
const { codeNavigationPath, blobPath, definitionPathPrefix } = codeNavEl.dataset;
// eslint-disable-next-line promise/catch-or-return // eslint-disable-next-line promise/catch-or-return
import('~/code_navigation').then(m => import('~/code_navigation').then(m =>
......
---
title: Fix jump to definition linking to same file opening a new tab
merge_request:
author:
type: changed
...@@ -23,17 +23,20 @@ exports[`Code navigation popover component renders popover 1`] = ` ...@@ -23,17 +23,20 @@ exports[`Code navigation popover component renders popover 1`] = `
<div <div
class="popover-body" class="popover-body"
> >
<gl-deprecated-button-stub <gl-button-stub
category="tertiary"
class="w-100" class="w-100"
href="http://test.com" data-testid="go-to-definition-btn"
size="md" href="http://gitlab.com/test.js#L20"
icon=""
size="medium"
target="_blank" target="_blank"
variant="default" variant="default"
> >
Go to definition Go to definition
</gl-deprecated-button-stub> </gl-button-stub>
</div> </div>
</div> </div>
`; `;
...@@ -48,6 +48,7 @@ describe('Code navigation app component', () => { ...@@ -48,6 +48,7 @@ describe('Code navigation app component', () => {
factory({ factory({
currentDefinition: { hover: 'console' }, currentDefinition: { hover: 'console' },
currentDefinitionPosition: { x: 0 }, currentDefinitionPosition: { x: 0 },
currentBlobPath: 'index.js',
}); });
expect(wrapper.find(Popover).exists()).toBe(true); expect(wrapper.find(Popover).exists()).toBe(true);
......
import { shallowMount } from '@vue/test-utils'; import { shallowMount } from '@vue/test-utils';
import Popover from '~/code_navigation/components/popover.vue'; import Popover from '~/code_navigation/components/popover.vue';
const DEFINITION_PATH_PREFIX = 'http:/'; const DEFINITION_PATH_PREFIX = 'http://gitlab.com';
const MOCK_CODE_DATA = Object.freeze({ const MOCK_CODE_DATA = Object.freeze({
hover: [ hover: [
...@@ -10,7 +10,7 @@ const MOCK_CODE_DATA = Object.freeze({ ...@@ -10,7 +10,7 @@ const MOCK_CODE_DATA = Object.freeze({
value: 'console.log', value: 'console.log',
}, },
], ],
definition_path: 'test.com', definition_path: 'test.js#L20',
}); });
const MOCK_DOCS_DATA = Object.freeze({ const MOCK_DOCS_DATA = Object.freeze({
...@@ -20,13 +20,15 @@ const MOCK_DOCS_DATA = Object.freeze({ ...@@ -20,13 +20,15 @@ const MOCK_DOCS_DATA = Object.freeze({
value: 'console.log', value: 'console.log',
}, },
], ],
definition_path: 'test.com', definition_path: 'test.js#L20',
}); });
let wrapper; let wrapper;
function factory(position, data, definitionPathPrefix) { function factory({ position, data, definitionPathPrefix, blobPath = 'index.js' }) {
wrapper = shallowMount(Popover, { propsData: { position, data, definitionPathPrefix } }); wrapper = shallowMount(Popover, {
propsData: { position, data, definitionPathPrefix, blobPath },
});
} }
describe('Code navigation popover component', () => { describe('Code navigation popover component', () => {
...@@ -35,14 +37,33 @@ describe('Code navigation popover component', () => { ...@@ -35,14 +37,33 @@ describe('Code navigation popover component', () => {
}); });
it('renders popover', () => { it('renders popover', () => {
factory({ x: 0, y: 0, height: 0 }, MOCK_CODE_DATA, DEFINITION_PATH_PREFIX); factory({
position: { x: 0, y: 0, height: 0 },
data: MOCK_CODE_DATA,
definitionPathPrefix: DEFINITION_PATH_PREFIX,
});
expect(wrapper.element).toMatchSnapshot(); expect(wrapper.element).toMatchSnapshot();
}); });
it('renders link with hash to current file', () => {
factory({
position: { x: 0, y: 0, height: 0 },
data: MOCK_CODE_DATA,
definitionPathPrefix: DEFINITION_PATH_PREFIX,
blobPath: 'test.js',
});
expect(wrapper.find('[data-testid="go-to-definition-btn"]').attributes('href')).toBe('#L20');
});
describe('code output', () => { describe('code output', () => {
it('renders code output', () => { it('renders code output', () => {
factory({ x: 0, y: 0, height: 0 }, MOCK_CODE_DATA, DEFINITION_PATH_PREFIX); factory({
position: { x: 0, y: 0, height: 0 },
data: MOCK_CODE_DATA,
definitionPathPrefix: DEFINITION_PATH_PREFIX,
});
expect(wrapper.find({ ref: 'code-output' }).exists()).toBe(true); expect(wrapper.find({ ref: 'code-output' }).exists()).toBe(true);
expect(wrapper.find({ ref: 'doc-output' }).exists()).toBe(false); expect(wrapper.find({ ref: 'doc-output' }).exists()).toBe(false);
...@@ -51,7 +72,11 @@ describe('Code navigation popover component', () => { ...@@ -51,7 +72,11 @@ describe('Code navigation popover component', () => {
describe('documentation output', () => { describe('documentation output', () => {
it('renders code output', () => { it('renders code output', () => {
factory({ x: 0, y: 0, height: 0 }, MOCK_DOCS_DATA, DEFINITION_PATH_PREFIX); factory({
position: { x: 0, y: 0, height: 0 },
data: MOCK_DOCS_DATA,
definitionPathPrefix: DEFINITION_PATH_PREFIX,
});
expect(wrapper.find({ ref: 'code-output' }).exists()).toBe(false); expect(wrapper.find({ ref: 'code-output' }).exists()).toBe(false);
expect(wrapper.find({ ref: 'doc-output' }).exists()).toBe(true); expect(wrapper.find({ ref: 'doc-output' }).exists()).toBe(true);
......
...@@ -173,7 +173,11 @@ describe('Code navigation actions', () => { ...@@ -173,7 +173,11 @@ describe('Code navigation actions', () => {
[ [
{ {
type: 'SET_CURRENT_DEFINITION', type: 'SET_CURRENT_DEFINITION',
payload: { definition: { hover: 'test' }, position: { height: 0, x: 0, y: 0 } }, payload: {
blobPath: 'index.js',
definition: { hover: 'test' },
position: { height: 0, x: 0, y: 0 },
},
}, },
], ],
[], [],
...@@ -193,7 +197,11 @@ describe('Code navigation actions', () => { ...@@ -193,7 +197,11 @@ describe('Code navigation actions', () => {
[ [
{ {
type: 'SET_CURRENT_DEFINITION', type: 'SET_CURRENT_DEFINITION',
payload: { definition: { hover: 'test' }, position: { height: 0, x: 0, y: 0 } }, payload: {
blobPath: 'index.js',
definition: { hover: 'test' },
position: { height: 0, x: 0, y: 0 },
},
}, },
], ],
[], [],
...@@ -214,7 +222,11 @@ describe('Code navigation actions', () => { ...@@ -214,7 +222,11 @@ describe('Code navigation actions', () => {
[ [
{ {
type: 'SET_CURRENT_DEFINITION', type: 'SET_CURRENT_DEFINITION',
payload: { definition: { hover: 'test' }, position: { height: 0, x: 0, y: 0 } }, payload: {
blobPath: 'index.js',
definition: { hover: 'test' },
position: { height: 0, x: 0, y: 0 },
},
}, },
], ],
[], [],
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment