|Subject:||Re: lisp/term/ns-win.el modification|
|Date:||Thu, 27 Apr 2017 13:30:26 +0200|
* All authors of a patch much assign the copyright to FSF in order for it to be included in Emacs. This mean that you can't include a random piece of code from the internet (in this case the `trim-spaces' function).Ok, but such a trimming function is trivial and I don't see how my rewriting it to make it different from that code would add to it. What should I do?
Comments on the patch itself:* I'm not exactly sure which problem you were trying to solve. I just tested to select a number of files in the Finder on macOS (10.10.5) and dragged then to Emacs (25.1), and they open just fine. Please include a description in the source code for the situation your code handle.Services are called from the Services menu, not by drag-and-dropping.The "Open Selected File" service takes a string as its argument, tries to parse that as a path on the file system and if a file corresponds to that path opens it, otherwise creates a buffer with the file name as its name.The service is basically used on text files, not in the Finder.
* I miss a description, or link to a description, of the format of `ns-input-spi-argument'. Without it it's hard to tell what the code is trying to do, and whether or not it does it correctly.You mean `ns_input_spi_arg', right? The line above (defvar ns-input-spi-name) refers to a `nsterm.m' file where it seems to be defined the following way:ns_input_spi_arg = build_string ([arg UTF8String]);
* The code will be smaller, and more easy to read, if you would use `dolist' instead of `while'. Also, if you do this change, I see no need to break out the code into a separate function, as it will only be two or three lines long.I don't want to modify the ns-spi-service-call block more than necessary.
* File names in macOS may start or end with spaces. It's probably not a good idea to trim spaces. (Again, it's hard to tell without a description of the format.)Yes, but we're not talking about file names but file paths in a text file here.The problem with not trimming is that when a legit file is tripple-clicked, the whole line is selected, including trailing spaces. The way the service is currently implemented considers that spaces a belonging to the file name and since that file does not exist just opens a blank buffer.
So I guess I could add a test to check wether the trimmed file exist and if not add the selected white space until I get an existing file (but then we'd hit cases where 2 files ending with different sorts of white space could be confused for each other).
|[Prev in Thread]||Current Thread||[Next in Thread]|