[PATCH] D118135: [demangler] preserve line numbering in copied sources

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added subscribers: tstellar, Quuxplusone, dblaikie.
ChuanqiXu added a comment.

@tstellar @dblaikie @Quuxplusone I want to ask if it is important to keep the filename in the file headers. I couldn't be sure about this. You guys looks experienced to answer the question than me.



================
Comment at: llvm/include/llvm/Demangle/ItaniumDemangle.h:3
-// See README.txt for instructions
-//===------------------------- ItaniumDemangle.h ----------------*- C++ -*-===//
 //
----------------
urnathan wrote:
> ChuanqiXu wrote:
> > Now the header name is missing, which is inconsistent with other headers. Is this intentional?
> Yeah, I went back and forth on that.  The 'Do not edit! really wants to be on the first line, and the emacs marker must be, to be effective.  One's left with either
> 1) Omit the 'see README.txt' instruction. this is kind of useful to humans to explain why not edit
> 2) Omit the file name, 
> 3) an overly-long line, yes, even though we have large displays, I'm still 80 column :)
> 
> I went with #2, we don't want this file manually edited anyway, is it important? 
> 
> WDYT?
> 
> [as an aside, I don't really understand the desire for a file name here at all -- it's only the local filename, not the path from root, so doesn't really uniquify the file.  But whatever.]
In fact, I am not sure if the filename in the header is important or not. I would add some other more experienced folks to ask their opinion.

I agree that it is benefit to add a new line to tell human to not edit the file. But I  don't think not every developer uses emacs (I use VSCode, for example). My point is that it is a little bit odd to do some change for a specific kind of users. I am not saying the work experience of emacs users is not important. I didn't say that.

I think another option would be add a blank line at the start of the original file and replace this line in the script.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118135/new/

https://reviews.llvm.org/D118135