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

Setting "Use spaces?" stopped working #7269

Closed
brsa opened this issue Mar 10, 2024 · 22 comments
Closed

Setting "Use spaces?" stopped working #7269

brsa opened this issue Mar 10, 2024 · 22 comments
Assignees
Milestone

Comments

@brsa
Copy link

brsa commented Mar 10, 2024

One more broken item in the Query Tool for v8.4 - a follow up to #7268

There is the option "Use spaces?"

Specifies whether or not to insert spaces instead of tabs when the tab key or auto-indent are used.

Used to work and insert spaces instead of tabs. Now the setting seems to be ignored and (unwanted) tabs are inserted.

@brsa brsa added the Bug label Mar 10, 2024
@adityatoshniwal adityatoshniwal self-assigned this Mar 11, 2024
adityatoshniwal added a commit that referenced this issue Mar 11, 2024
1. Replace option in query tool edit menu is not working on non-Mac OS. #7262
2. Format SQL shortcut and multiline selection are not working. #7268
3. "Use Spaces?" Preference of Editor is not working. #7269
@adityatoshniwal adityatoshniwal added this to the 8.5 milestone Mar 11, 2024
@adityatoshniwal adityatoshniwal removed their assignment Mar 11, 2024
@casparsch
Copy link

I have the same experience.

Some additional information:
If a tab-character is selected and again the tab-key is pressed this tab is expanded to spaces.
Furthermore the tabs-to-spaces is underneath the "SQL Formatting". This could be interpreted as applicable only when: "Format SQL" (Ctrl-Shift-K) is invoked. It is a subtle difference but I assume one does not want to distinguish between editing and formatting a query.
Found the property (tabs-to-spaces) only being used when looking into the pgAdmin code on the event of the "Format SQL". At least for what I understood from it. On no other places I could find this property.

@pravesh-sharma
Copy link
Contributor

The issue is still reproducible on the latest snapshot build.

@pravesh-sharma pravesh-sharma removed their assignment Mar 22, 2024
@adityatoshniwal
Copy link
Contributor

The issue is still reproducible on the latest snapshot build.

I couldn't reproduce this on latest source code. Attaching screen recording.

Screen.Recording.2024-03-23.at.11.28.11.mov

@pravesh-sharma
Copy link
Contributor

Attaching behaviour from version 8.3 if use spaces? is enabled pressing backspace used to clear space only right now it is clearing entire tab character. On pressing the arrow keys it works.

Screen.Recording.2024-03-23.at.12.06.02.PM.mov

@adityatoshniwal
Copy link
Contributor

Attaching behaviour from version 8.3 if use spaces? is enabled pressing backspace used to clear space only right now it is clearing entire tab character. On pressing the arrow keys it works.

Screen.Recording.2024-03-23.at.12.06.02.PM.mov

Are you expecting to replace all the tabs with spaces when "Use spaces" is used? If yes, then that's absurd. Once a tab is pressed, on backspace a tab will be removed. Use spaces means from here on it will use spaces for tabs.

@pravesh-sharma
Copy link
Contributor

Attaching behaviour from version 8.3 if use spaces? is enabled pressing backspace used to clear space only right now it is clearing entire tab character. On pressing the arrow keys it works.
Screen.Recording.2024-03-23.at.12.06.02.PM.mov

Are you expecting to replace all the tabs with spaces when "Use spaces" is used? If yes, then that's absurd. Once a tab is pressed, on backspace a tab will be removed. Use spaces means from here on it will use spaces for tabs.

No. I have attached only the previous behaviour and if the current one is the correct then we can ignore.

@adityatoshniwal
Copy link
Contributor

adityatoshniwal commented Mar 23, 2024

No. I have attached only the previous behaviour and if the current one is the correct then we can ignore.

I just tested v8.2, it has the same behaviour like the new. No matter use spaces is enabled or not, existing tabs are not replaced with spaces even in old.

@pravesh-sharma
Copy link
Contributor

No. I have attached only the previous behaviour and if the current one is the correct then we can ignore.

I just tested v8.2, it has the same behaviour like the new. No matter use spaces is enabled or not, existing tabs are not replaced with spaces even in old.

I am not saying it should replace the existing tabs with space. I am talking about behaviour on the backspace. We can connect offline and I can show you what exactly I mean.

@pravesh-sharma pravesh-sharma self-assigned this Mar 25, 2024
@pravesh-sharma
Copy link
Contributor

The issue has been resolved. Verified on snapshot build.

@brsa
Copy link
Author

brsa commented Apr 12, 2024

While version 8.5 now inserts spaces when pressing the [tab] key as set up in the preferences (thank you!), some issues remain:

  1. With "Use spaces?" enabled, pressing [tab] always inserts a constant number of spaces (3 in my case).

  2. On a new line, or with only tabs and spaces before the cursor, pressing [backspace] deletes back a variable number of spaces (or one tab character) to align with the previous tab position. That changes after the first non-space character in the line: then [backspace] deletes back exactly one character.

This is the exact opposite of how it should be:

  1. Pressing [tab] should insert space characters to align on the next multiple of tab width.

  2. Pressing [backspace] should always delete exactly one character. No matter what.

@adityatoshniwal
Copy link
Contributor

While version 8.5 now inserts spaces when pressing the [tab] key as set up in the preferences (thank you!), some issues remain:

  1. With "Use spaces?" enabled, pressing [tab] always inserts a constant number of spaces (3 in my case).

Can't reproduce. It adds spaces based on preference.

  1. On a new line, or with only tabs and spaces before the cursor, pressing [backspace] deletes back a variable number of spaces (or one tab character) to align with the previous tab position. That changes after the first non-space character in the line: then [backspace] deletes back exactly one character.

This is the exact opposite of how it should be:

  1. Pressing [tab] should insert space characters to align on the next multiple of tab width.
  2. Pressing [backspace] should always delete exactly one character. No matter what.

This is a default behaviour in most of standard IDEs. I guess we'll need to add more preferences for Editor. Check my comment here - #7317 (comment)

@brsa
Copy link
Author

brsa commented Apr 26, 2024

While version 8.5 now inserts spaces when pressing the [tab] key as set up in the preferences (thank you!), some issues remain:

  1. With "Use spaces?" enabled, pressing [tab] always inserts a constant number of spaces (3 in my case).

Can't reproduce. It adds spaces based on preference.

Pressing [tab] should align at the next tab position - whether with a single tab character or the required number of spaces. Currently, with "Use spaces?" enabled, a fixed number of spaces is entered, alignment is not observed. This is just wrong. But maybe the work done in #7317 also takes care of this? I'll have to hope for version 8.6 ...

@adityatoshniwal
Copy link
Contributor

@brsa You don't need to wait for the 8.6 release to test this. You can test with daily nightly builds generated here - https://www.postgresql.org/ftp/pgadmin/pgadmin4/snapshots/

@casparsch
Copy link

casparsch commented May 3, 2024

While version 8.5 now inserts spaces when pressing the [tab] key as set up in the preferences (thank you!), some issues remain:

  1. With "Use spaces?" enabled, pressing [tab] always inserts a constant number of spaces (3 in my case).
  2. On a new line, or with only tabs and spaces before the cursor, pressing [backspace] deletes back a variable number of spaces (or one tab character) to align with the previous tab position. That changes after the first non-space character in the line: then [backspace] deletes back exactly one character.

This is the exact opposite of how it should be:

  1. Pressing [tab] should insert space characters to align on the next multiple of tab width.
  2. Pressing [backspace] should always delete exactly one character. No matter what.

Waited for release 8.6 to verify.
This issue is actually already mentioned in release 8.5, to be resolved. I missed it.
As I would expect the behavior as quoted this is not the case in 8.6. So also #7317 did not resolve it
That 7317 is about removing space (backspace) this issue is on adding upto the next tab-position.
Question is:

  • Should I create a new issue
  • Can this be reopened

@akshay-joshi
Copy link
Contributor

@casparsch

v8.6 has already been released yesterday.

@casparsch
Copy link

@casparsch

v8.6 has already been released yesterday.

I know, I already installed it. As also stated in my comment: "... this is not the case in 8.6."

@brsa
Copy link
Author

brsa commented May 5, 2024

With "Use spaces?" activated, pressing [tab] does not align at the next tab position, as reported. The bug is still present in version 8.6.

Reopen this one or create a new issue?

@adityatoshniwal
Copy link
Contributor

With "Use spaces?" activated, pressing [tab] does not align at the next tab position, as reported. The bug is still present in version 8.6.

Reopen this one or create a new issue?

Hi,
Can you explain what exactly is the issue? May be with some screenshot or recording?

@brsa
Copy link
Author

brsa commented May 6, 2024

It's very simple. How can there be misunderstanding?
Pressing the [tab] key does not align at the next tab position, which is the actual purpose of the [tab] key.

To reproduce, activate "Use spaces?" with "Tab size" = 3 (for example). Then, in the query tool:

  • Press [tab] → 3 spaces are entered
  • On the next line, enter 1 character, then press [tab] → still 3 spaces are entered, but it should be 2 spaces to align at position 3
  • On the next line, enter 2 characters, then press [tab] → still 3 spaces are entered, but it should be 1 space to align at position 3
-- expected:
   |
1  |
12 |
123   |
1234  |

-- status quo:
   |
1   |
12   |
123   |
1234   |

I am running Ubuntu Ubuntu 20.04.6 LTS. This is what "About pgAdmin4" dumps:

Version 8.6
Application Mode Desktop
Current User pgadmin4@pgadmin.org
NW.js Version 0.77.0
Browser Chromium 114.0.5735.91
Operating System Linux-5.15.0-105-generic-x86_64-with-glibc2.29
pgAdmin Database File /home/erwin/.pgadmin/pgadmin4.db
Log File /home/erwin/.pgadmin/pgadmin4.log

@adityatoshniwal
Copy link
Contributor

adityatoshniwal commented May 6, 2024

It's very simple. How can there be misunderstanding?

Probably because English is not my first language and don't hold any literature degree in English.

@adityatoshniwal
Copy link
Contributor

@brsa Anyway, you can create a separate ticket with details you just mentioned.

@casparsch
Copy link

Maybe this will clarify things: tabs (history)
By reading the first paragraph I think it supports what brsa stated.

Yes, it at least goes back to the typewriter era, like other concepts (at least the "carriage", "return", "linefeed" and even the "bell"). The bell (physically present) was to signal the typist he or she was near the end of the line.
So tabs back then were a specific position for the carriage to slide to. These positions could arbitrarily selected/set by the typist.

In case of editors for coding nowadays it is widely accepted (even expected) to have the cursor going to the next position to the multiple of "the number the spaces" as configured (the width of the tab).
For instance take a look at editors like Notepad++ (windows), geany (linux), sublime (linux). Even Notepad from microsoft itself, though it does not have the option to fill with spaces or set the width, it aligns to the same position on the horizontal line.

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

No branches or pull requests

5 participants