Alexey-T / CudaText

Cross-platform text editor, written in Free Pascal
Mozilla Public License 2.0
2.52k stars 174 forks source link

Feature request: Add option to not unwrap symlinks when editing files or directories from the command line. #4405

Closed bogen85 closed 1 year ago

bogen85 commented 2 years ago

I'm using CudaText installed via flatpak, io.github.cudatext.CudaText-Qt5, version 1.171.0 but this likely applies to all installation methods. (on up to date ArchLinux x86_64).

When editing files (which open as new tabs) or directories (which are treated as a node in a project) that are directory that has a symlink in the path, the symlink is resolved and the unwrapped path is the one that CudaText displays and uses.

I often use symlinks to either:

  1. make things consistent on different setups
  2. to shorten the complete path
  3. make the path more applicable to what I'm currently working on.

Since the symlinks are fully resolved and unwrapped, I lose all the above desired features that I'm trying to leverage from the use of symlinks.

Linux and other *nix do not care which path is presented a low level. The path with symlinks, or the resolved unwrapped absolute path, both work the same.

Since they both work the same, it would be useful to have the option to preserve symlinks.

When the current working directly is a path with symlinks it it, and Linux is queried for the current working directory, the symbolic path is returned, not the absolute path. Typically an additional step needs to be done to resolve and unwrap the symbolic path and get the absolute path.

So I'd like to have an option to use the symbolic path as is in all that CudaText displays concerning the current project directory and the paths associated with the files currently open in the tabs.

I don't see this as a current option.

Usage:
  cudatext [ key ... ] filename ...

Supported keys:
  -h, --help      - Show this help and exit
  -v, --version   - Show application version and exit
  -z=[text|binary|hex|unicode|uhex] - Open arguments in internal viewer
  -r              - Open arguments in read-only mode
  -e=value        - Open arguments in given encoding
  -el             - Show supported encoding names, exit
  -n              - Ignore option "ui_one_instance", open new app window
  -nh             - Ignore saved file history
  -nsl            - No session loading on start
  -nss            - No session saving on exit
  -ns             - The same as -nsl and -nss
  -nn             - Don't suggest to create new file if filename not found
  -s=folder       - Set full path of "settings" folder
  -i              - Open contents of stdin in new tab (Unix only)
  -verbose        - Copy Python messages to stdout (Unix only)
  -id=name - Set single-instance id, for groups of instances (Unix, default: cudatext.0)
  -w=left,top,width,height        - Set position/size of the main window
  -c=cuda_module,method           - Run command plugin on startup
  -p=cuda_module#param1#param2... - Run event "on_cli" on startup

Filenames can be with ":line" or ":line:column" suffix to place caret.
Folder can be passed, will be opened in Project Manager plugin.
Projects (*.cuda-proj) can be passed, will be opened in Project Manager.
Sessions (*.cuda-session) can be passed, even without Session Manager plugin.
Alexey-T commented 2 years ago

I may try to add an option. option in the user.json or command-like key? how other text editors do handle this option?

bogen85 commented 2 years ago

user.json is fine as well. That would simplify things. Let me check on other editors.

bogen85 commented 2 years ago

gedit does not give any options for this, but it preserves the symbolic path. mousepad same, it preserves the symbolic path, did not find an option to control. geany same, it preserves the symbolic path, did not find an option to control.

emacs preserves the symbolic path, I did not dig through options...

gvim converts the symbolic path to an absolute one, I looked but could not find an option to change this behavior. sublime text converts the symbolic path to an absolute one, I looked through the options for it as well, did not find one to change this behavior. VS code same as sublime text, converts path, looked for but did not find option to change the behavior. kate same as sublime text, converts path, looked for but did not find option to change the behavior.

bogen85 commented 2 years ago

does not need to be a command line option. Since I prefer symbolic paths to be preserved, I don't see why I'd ever want that to be overridden on a particular file or directory from the command line.

In fact, having it in user.json just would make it easier overall, I would not have to specify from the command line either way.

Alexey-T commented 2 years ago

Ubuntu 20

-> all I need - is disable resolving for lazarus Open File/Folder dialogs. easy to fix. I don't want the option here.

bogen85 commented 2 years ago

@Alexey-T I don't completely understand what you mean.

Are you saying for you opening filenames and folders from the command line does not convert the symbolic paths to absolute paths for you?

Or are you saying that the you to change it so does not convert the paths? And no option, just not convert?

Alexey-T commented 2 years ago

If I run from terminal

$ cudatext file_symlink.css

then Cud opens the file_symlink.css and does NOT resolve the symlink. App title and statusbar shows symlink name.

If I do File Open dialog and choose this file_symlink.css then dialog resolves the filename to ~/tests/main.css and Cud shows resolved filename.

do you confirm it? I see it for gtk2 and qt5 versions. so I suggest only to disable resolving in the File Open dialog.

bogen85 commented 2 years ago

Hmm... Interesting. Let me see if this is a flatpak issue, I'll try a non flatpak CudaText and report back.

(The command line behavior you are observing is not the command line behavior I'm observing).

Alexey-T commented 2 years ago

Flatpak issue? it's possible.

bogen85 commented 2 years ago

I tried CudaText from http://www.uvviewsoft.com/cudatext/files_linux/cudatext-linux-qt5-amd64-1.171.0.0.tar.xz

It also converts the symbolic path to an absolute one. (From the command line)

All of my tests with the other editors were from the same terminal and shell session, so I can't imagine anything in my shell (bash) causing this behavior.

Alexey-T commented 2 years ago

then I don't know why you got resolved filename. Cud's source codde do NOT have this resolving except in one place (processing of cudatext original binary filename to check is it symlink).

Alexey-T commented 2 years ago

please test it on another PC and/or Linux distro. I have Ubuntu 20.

bogen85 commented 2 years ago

And I know it is not using the same binary (the one from tarball vs the flatpak one, as all the settings are different, plus I'm executing via /usr/bin/cudatext and not via script that uses flatpak run ...)

And no, my script is not expanding it... (I put echo in from of the flatpak run ...) but let me add pwd to the script.

And I'll check from different distros if it is not the script, AlmaLinux 8 and 9, and Fedora.

However, I did not use the binary from the tarball via a script, so I doubt that is the issue.

Alexey-T commented 2 years ago

my test.

symlink is NOT resolved in app title and ui-tab tooltip

bogen85 commented 2 years ago

For me I ruled out the script. pwd in the script is the symbolic path.

Alexey-T commented 2 years ago

so your script did resolve the path?

bogen85 commented 2 years ago

Script did not resolve the path. It keeps it as is, as the symbolic path. It does NOT convert/resolve it.

bogen85 commented 2 years ago

I'm trying other distros now.

Alexey-T commented 2 years ago

https://github.com/Alexey-T/CudaText/issues/4405#issuecomment-1265999957 my test, can u repeat it?

bogen85 commented 2 years ago

I extracted http://www.uvviewsoft.com/cudatext/files_linux/cudatext-linux-qt5-amd64-1.171.0.0.tar.xz into a folder that is in a symbolic path.

I created a file in the that path. I ran the extracted cudatext binary on the file, it converted the path to absolute.

wget http://www.uvviewsoft.com/cudatext/files_linux/cudatext-linux-qt5-amd64-1.171.0.0.tar.xz
mkdir x
cd x
tar Jxvf ../cudatext-linux-qt5-amd64-1.171.0.0.tar.xz
cd ..
ps aux > ps-aux.txt
./x/cudatext ./ps-aux.txt 

I set { "ui_title_path": true, } to see the path.

I also tried:

cd x
ls
cp ../ps-aux.txt .
./cudatext ps-aux.txt 

Same thing. (symbolic path is converted to an absolute path)

I installed DoubleCommander. Created a symlink to a directory. I repeated the test. (in that symlink). Same results.

Ok, I'm not talking about symlinked files. I'm talking about directories that have a symlink somewhere in the parent path.

When I opened symlinked files, like you indicated your test, the path of the symlinked file is NOT converted.

Alexey-T commented 2 years ago

so you have dir with Cud, which has a symlink? that is the reason. Cud converts its program's dir to resolved path. that was required to run Cud by 'cudatext' symlinked to another name.

to disable this resolving, user.json option will be not ok, so I sugggest the command-line key like -nl.

Alexey-T commented 2 years ago

when resolving symlink path, Cud must write to terminal:

'CudaText starts via symlink to: ......'

do you see it?

bogen85 commented 2 years ago

I agree CudaText needs to resolve it's program directory.

I'm talking about the directory of the files I'm working in, and the directory my shell is in when I'm launching cudatext, not the the directory the cudatext binary is in.

The cudatext binary would be in a completely different branch of a parent path in that tree, and not usually one that is symlinked.

I'm not seeing any such output in the terminal or python console output.

Even if I use the full path to CudaText (in a symlinked path) from the command line to edit the file.

Alexey-T commented 2 years ago

is this the OK test now?

bogen85 commented 2 years ago

Ok, I'll test. But I'm not putting Cud in a symlinked directory for installation... (If my package manager or flatpak does, that is not my concern) The working directory for the projects (with source I'm edittng with Cud) is in a symlinked directory.

$ mkdir -p ~/tests/ddd
$ ln -sf ~/tests/ddd ~/dir_sym
$ cd ~/dir_sym
$ tar Jxf /path/to/cudatext-linux-qt5-amd64-1.171.0.0.tar.xz 
$ cp readme/history.txt torrent.cpp
$ ./cudatext torrent.cpp 

And, yes, what you are saying is correct:

-> Cud shows for ui-tab hint: ~/tests/ddd/torrent.cpp but I want ~/dir_sym/torrent.cpp

Alexey-T commented 2 years ago

Cud shows for ui-tab hint: ~/tests/ddd/torrent.cpp but I want ~/dir_sym/torrent.cpp

but I think that resolving is done by Shell here. ie when we run './cudatext', shell expands '.'. Cud must not do it! not sure here

Alexey-T commented 2 years ago

not sure how to determine who does the 'dot' expansion in './cudatext'.

Alexey-T commented 2 years ago

made the simple app which shows the ParamStr(0) ie the self folder, in the form caption. it shows the resolved path. so seems the Shell does the resolve.

para

bogen85 commented 2 years ago

I took a different approach...

$ cd /home/shared-development/
$ mkdir -pv abc/def/ghi
mkdir: created directory 'abc'
mkdir: created directory 'abc/def'
mkdir: created directory 'abc/def/ghi'
$ ln -sv abc/def/ghi jkl
'jkl' -> 'abc/def/ghi'
$ cd jkl
$ cp ~/dir_sym/getcurdir*.cxx .
$ cp ~/dir_sym/getcurdir*.pas .
$ ls
getcurdircxx.cxx  getcurdirpas.pas
$ fpc getcurdirpas.pas 
Free Pascal Compiler version 3.2.2 [2022/08/31] for x86_64
Copyright (c) 1993-2021 by Florian Klaempfl and others
Target OS: Linux for x86-64
Compiling getcurdirpas.pas
Linking getcurdirpas
8 lines compiled, 0.1 sec
$ clang++ -o getcurdircxx getcurdircxx.cxx 
$ ./getcurdirpas 
Current directory is : /home/shared-development/abc/def/ghi
$ ./getcurdircxx 
Current working directory is: /home/shared-development/jkl

getcurdirpas.pas

program getcurdirpas;
var
  s: string;

begin
  getDir (0, s);
  writeln ('Current directory is : ', s);
end.

getcurdircxx.cxx

#include <unistd.h>
#include <cstdio>

int main() {
  printf("Current working directory is: %s\n", get_current_dir_name());
}

The pascal one converted the path from symbolic to absolute. The c++ one left it unconverted.

bogen85 commented 2 years ago

And as I said before, the following I'm executing the same way, from my shell:

gedit does not give any options for this, but it preserves the symbolic path.
mousepad same, it preserves the symbolic path, did not find an option to control.
geany same, it preserves the symbolic path, did not find an option to control.
emacs preserves the symbolic path, I did not dig through options...
bogen85 commented 2 years ago

getcurdirpas2.pas

program getcurdirpas2;

uses sysutils;

var
  s: string;

begin
  s := sysutils.getcurrentdir;
  writeln ('Current directory is : ', s);
end.
$ fpc getcurdirpas2.pas 
Free Pascal Compiler version 3.2.2 [2022/08/31] for x86_64
Copyright (c) 1993-2021 by Florian Klaempfl and others
Target OS: Linux for x86-64
Compiling getcurdirpas2.pas
Linking getcurdirpas2
11 lines compiled, 0.2 sec
$ ./getcurdirpas2 
Current directory is : /home/shared-development/abc/def/ghi
$ ./getcurdircxx 
Current working directory is: /home/shared-development/jkl
$ pwd
/home/shared-development/jkl
bogen85 commented 2 years ago

So, it is not the shell I'm using in Linux...

bogen85 commented 2 years ago

So...

Who should address this at fpc gitlab, you or me?

I guess since I reported this issue here... Maybe that falls on me...

bogen85 commented 2 years ago

. (dot) is not expanded by the shell That is likely something in fcl that is doing that...

bogen85 commented 2 years ago

Yeah, it is fcl

getcurdirpas2.pas

program getcurdirpas2;

uses sysutils;

var
  s: string;

begin
  s := sysutils.getcurrentdir;
  writeln ('Current directory is : ', s);

  writeln ('"." expanded is ', expandFileName('.'));
end.

That results in:

$ fpc getcurdirpas2.pas 
Free Pascal Compiler version 3.2.2 [2022/08/31] for x86_64
Copyright (c) 1993-2021 by Florian Klaempfl and others
Target OS: Linux for x86-64
Compiling getcurdirpas2.pas
Linking getcurdirpas2
13 lines compiled, 0.2 sec
$ pwd
/home/shared-development/jkl
$ ./getcurdirpas2
Current directory is : /home/shared-development/abc/def/ghi
"." expanded is /home/shared-development/abc/def/ghi
$ pwd
/home/shared-development/jkl
$ ./getcurdircxx 
Current working directory is: /home/shared-development/jkl

Definitely not the shell doing this...

bogen85 commented 2 years ago

I will follow up in fpc gitlab and reference this issue. (with a link to the post here where it starts to get relevant from an fpc standpoint).

I'll update this issue #4405 with the link to the issue I file at fpc gitlab...

bogen85 commented 2 years ago

Unless you have a better idea...

bogen85 commented 2 years ago

getcurdircxx.cxx

#include <unistd.h>
#include <cstdio>

int main(int argc, char **argv) {
  printf("Current working directory is: %s\n", get_current_dir_name());
  printf("argv[0] is %s\n", argv[0]);
}

getcurdirpas2.pas

program getcurdirpas2;

uses sysutils;

var
  s: string;

begin
  s := sysutils.getcurrentdir;
  writeln ('Current directory is : ', s);

  writeln ('"." expanded is ', expandFileName('.'));
  writeln ('ParamStr(0) is ', ParamStr(0));
end.

Output from those:

$ clang++ -o ./getcurdircxx getcurdircxx.cxx 
$ ./getcurdircxx 
Current working directory is: /home/shared-development/jkl
argv[0] is ./getcurdircxx
$ fpc getcurdirpas2.pas 
Free Pascal Compiler version 3.2.2 [2022/08/31] for x86_64
Copyright (c) 1993-2021 by Florian Klaempfl and others
Target OS: Linux for x86-64
Compiling getcurdirpas2.pas
Linking getcurdirpas2
14 lines compiled, 0.2 sec
$ ./getcurdirpas2 
Current directory is : /home/shared-development/abc/def/ghi
"." expanded is /home/shared-development/abc/def/ghi
ParamStr(0) is /home/shared-development/abc/def/ghi/getcurdirpas2
$ pwd
/home/shared-development/jkl

Once again, clearly not the shell.

Alexey-T commented 2 years ago

correct, it is the FPC issue. so let's report it there, FPC Gitlab. pls do.

Alexey-T commented 2 years ago

your last FPC app is best - show them 3 result strings.

bogen85 commented 2 years ago

Ok, I filed it as feature request, because filing it as bug likely does not make sense (for reasons explained in the issue report). FPC source issue # 39936: Want a way to get current current working directory (or any directory) without resolving symlinks

bogen85 commented 2 years ago

https://gitlab.com/freepascal.org/fpc/source/-/issues/39936#note_1123171495 Was immediately closed by Michael Van Canneyt....

Initially I did not agree with his reasoning...

bogen85 commented 2 years ago

However, he has a point.

$ ./getcurdircxx 
Current working directory is: /home/shared-development/jkl
argv[0] is ./getcurdircxx

$ PWD= ./getcurdircxx 
Current working directory is: /home/shared-development/abc/def/ghi

Wow... get_current_dir_name in C is fragile... (but at least it gave the correct absolute path when PWD was unset).

Not sure how to go about this. I guess since this is launched form the user's shell one would trust the user to not set PWD maliciously.

Ideally there would unit in fcl to address this, and it is unlikely Michael would accept a third party unit for this any time soon...

It would not be hard to in Linux (and any other currently applicable nix, BSD or MacOS) using free pascal to implement a robust get_current_dir_name equivalent that also verifies that the value of the environment variable PWD and the expanded absolute path and a get current working directory function that does not use PWD are the same...

I'm going to need such a unit for what I'm working on... If I were to develop that and prove it (unit tests) would that be something CudaText could use to address this issue? (a copy of this unit would be included in the CudaText source code for it to use.)

I'd be willing to do the leg work on CudaText, adding the command line switch, initially Linux only, to address this issue #4405 , and create a PR.

However, that, doing the leg work also of course is contingent on how the path expansion is handled or not handled throughout the CudaText code base... Especially if it relying on fcl and lcl calls for this throughout the code... I would think such a PR would need to not touch a lot of existing CudaText units. Otherwise work for this PR work is too involved and potentially risky for CudaText based on my limited knowledge of the CudaText code base. I use CudaText. It works. Apart from an occasional non-invasive fix needed, I don't need to study the CudaText code and layout.

But I'd need to know ahead of time is this is something you would accept once we get the kinks ironed out... (the PR that is). The unit I describe above I'll be needing to do regardless if such a PR is accepted by you for CudaText

Alexey-T commented 2 years ago

If I were to develop that and prove it (unit tests) would that be something CudaText could use to address this issue?

it depends on how complex this unit will be. and how much changes in Cud we need. if we need to fix only this func--- proc_files.pas

function AppExpandFilename(const fn: string): string;
begin
  if fn='' then exit(fn);

  //handle cmd-line options here
  if fn[1]='-' then exit(fn);

  Result:=
    ResolveWindowsLinkTarget(
    ExpandFileName(
    {$ifdef windows}
    AppExpandWin32RelativeRootFilename(
    {$else}
    AppExpandHomeDirInFilename(
    {$endif}
    fn
    )));
end;

then maybe Ok.

bogen85 commented 2 years ago

The unit (to be developed by me, which I need anyways) will only be a few functions that are a few lines each. I'll let you know when I've completed that. The tests will be more complex then those...

The functions are:

  1. Retrieve current directory like C/C++ do, but with some checking/validating of the PWD env far to insure what is returned is valid and is actually the current directory, and in worst case, just return the expanded path for the current directory.
  2. A relative to non relative path converter, that takes into account the possibility of current directory being a symbolic path, and worst case as for function 1, just return the expanded full path for the relative path.
  3. A function to retrieve that first program argument as it was issued from the command line. (which Cud won't need).

Only what is needed by Cud would be in the PR.